Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Reorganize In-App Dashboard & Use New Energy Footprint Calculations #1176

Merged
merged 51 commits into from
Sep 22, 2024

Conversation

JGreenlee
Copy link
Collaborator

@JGreenlee JGreenlee commented Sep 9, 2024

Highlights / High level summary of changes

  • New dashboard UI, which uses the the footprint calculations from e-mission-common (aka CHEER)

    Footprint section

    Weekly emissions & energy use can be broken down by mode (theoretically purpose as well, if configured)

    Movement section

    Shows however many weeks are loaded, rather than just the past 2 weeks (although the default initial load is still only 2 weeks)

    Travel section

    No major changes besides UI


Miscellaneous

  • Timeline always initializes to the last 7 days. There is a shortcut to jump to the last processed week if the timeline is empty (useful for testing historical data)

  • SnackBars no longer block interaction with the UI while visible

  • AppBar follows Material Design 3 guidelines. StatusBar matches as well. Date display is more user-friendly and less cluttered

    image

Move the components used by a 'row' of cards to a subdirectory of 'metrics'
Create new components for each section which groups the cards into a Carousel
Renamed "Summary" section to "Travel" section; this will show 'distance', 'duration', and 'count'

Update types to include 'footprint' as a metric, and revise the sections ('footprint', 'movement', 'surveys', 'travel')
- update color palette to align with Material Design 3 (which is what React Native Paper is based on)
- update NavBar component; pass props through to the underlying Appbar.Header, allowing flexibility of elevated = true or false
includes a fix for 'validateDOMNesting' error message
Moved to a more general location since this can be used in many parts of the UI
height = 40 & adjust padding/margins to match standard Material UI button
use lighter grey

on DateSelect, show on one line and use month + day (e.g. "Sep 9") instead of MM/DD/YYYY
Speeds up development; does not affect production
We had used <Modal> here (while still in the process of migrating from Angular) to ensure the snackbar shows above the rest of the content.

This prevented interacting with other content until the snackbar (and its modal) were dismissed.

Per RN Paper docs, Snackbars are intended to work with Portals. https://callstack.github.io/react-native-paper/docs/components/Snackbar/
We can do this now that we have a PaperProvider at the App level (index.js)
Bars on a chart get shown with transparency if they begin with this. In a previous version, it was dashed lines, but it's no longer apt to call it dash_key.
This mostly just matters for: 1) testing with historical data, or 2) people whose pipeline is stuck, which indicates a deeper problem.
But it will prevent any scenarios where several weeks/months get loaded in all at once.

To aid with scenario (1), we can add a button to the TimelineScrollList: if there is no travel in the past week and the pipeline end is not in the last week, this gives us a shortcut to the last processed week
both Label and Dashboard tabs use date & time formatting functions using ISO strings, including with/without weekday, and with/without year, and "humanized" duration strings
Makes generic versions of these and puts them in js/util.ts

DateSelect will use this as well; as a result it will shift from using a numerical, with year (e.g. "9/17/2024") representation, to an abbreviated string form (e.g. "Sep 17")
This is 1) more friendly / less initimidating, 2) allows the datepicker text to be one line, which reduces clutter
If there is an long stretch of time with no data, this implementation will not work; it should keep inserting empty weeks until there are no more days
Categorizing this as "Movement" instead of "Active Travel"
Also updating to determine the list of "active modes" from the presence of "met" in the rich modes.
Before, this was a config option or defaulted to only "walk" and "bike"
(now it will include "ebike" if "ebike" has non-zero "mets", or any other modes with non-zero "mets")

-update styles on the cards
When the user first goes to the Dashboard in an app session, we want to show them at least 2 weeks so they can compare the previous week to the past week.
But after that we should allow them to select any range.
So we can add an 'isInitialized' bool state to mark this, and only do this ">= 14 days" check on the initialization

Also add a 'refresh' function which not only refreshes the timeline but also resets 'isInitialized' (along with 'aggMetricsIsLoading')
and fix them since they were flipped ('low' should come first)
e-mission-common 0.6.1 comes with expanded metrics (support for 'footprint', which needs label_options passed from MetricsTab.tsx).

Update types: new LabelOptions spec, explicit type for RichMode, add 'footprint' to metricsTypes, add "MetricEntry" type which is like DayOfMetricData but not necessarily for 1 day (it could be aggregated across an entire week, for example)
Add "goals" to appconfig.metrics.phone_dashboard_ui.footprint_options
Also fixes translation issue: instead of checking for hardcoded value "Unlabeled" (which probably did not work in other languages) check if the label starts with whatever the "unlabeled" string is for the current language
Shows barchart of average daily footprint for the user, grouped by weeks, colored based on goals ("meter" of green -> red)
Includes checkboxes to break down by a grouping field (mode_confirm, purpose_confirm, etc)
If mode_confirm, then base mode colors are used instead of the 'meter'
Works for either 'carbon' (with kg_co2) or 'energy' (with kwh)

new functions in metricsHelper
- aggMetricEntries combines/ merges array of MetricEntry (used by WeeklyFootprintCard to aggregate multiple days into a week)
- sumMetricEntry adds up all values within a day
- sumMetricEntries does both

The type definitions may look complex here, but all they really do is allow the return type to depend on the 'metricName'. For example, if metricName is 'duration', then the return type is just a number. But if metricName is 'footprint', it is an object containing 'kwh' and 'kg_co2'
This way we don't have to write separate functions for every metric
use the new components, SummaryCard and WeeklyFootprintCard, on the Footprint section. Each of these work for both carbon and energy, so we don't need the 'energy' components, nor the old CarbonFootprintCard and CarbonTextCard

A few things of note in FootprintSection:

cumulativeFootprintSum sums the footprint metric (which includes both carbon and energy) across all days. This gets fed into SummaryCard

Goals are read from the appconfig. Because they are internationalized, though, the labels have to looked up by lang. If a footnote is configured, it is added and a reference to it is appended to the label.
The goals are passed to SummaryCard and WeeklyFootprintCard

A dynamic footnotes mechanism via `addFootnote`. Instead of including the footnote numbers, (e.g. ¹ ²) directly in the strings, we can round them up for the whole section, keeping track of their numbers, and show them all at the bottom.
This allows footnotes to be dynamically shown. If one configuration doesn't use "goals", then that footnote won't show. The "unlabeled" footnote will then become ¹, avoiding any weird scenarios where ¹ is not present and it skips straight to ²
We can expand on this later if we want to be more detailed about footnotes, references, data sources
used by WeeklyFootprintCard
instead of setting dateRange only once pipelineRange is set, we can set dateRange upfront and just not do anything with it until pipelineRange has been set

This saves us from constantly checking dateRange != null anywhere it is used downstream
It needs to be MultilabelKey by default, otherwise we have to specify this everywhere
Unifies the styling of all metrics cards

Adjusts navbar and refresh button on label screen so the label screen and metrics screen navbars match each other
Where base modes were previously a property of the label options, and had a separate set of properties, we now have rich modes that inherit from base modes and may include other properties.
This means for the numerous places in the UI where we use mode colors, we should always check the rich mode via get_rich_mode / get_rich_mode_for_value.

Also we can tighten down typings a bit more.
"text" does not exist in label options or in rich modes. It had been added in while label options are being read and the translations are parsed
Instead of that let's just have a function that does this (labelKeyToText). We need to check for translations in 1) the label options being used; and 2) the default label options. if nothing, then convert it to readable (convert underscores to spaces & capitalize)
We'll also have a function that does the opposite.

Updated some typings: LabelOptions type should not accept a parameter. Also should mark REPLACED_MODE as optional

UserInputData should not have "name". This only exists on EnketoUserInputData; it represents the name of the survey
not just the last 14 days
give default argument to MetricEntry and DayOfMetricData so we don't have to specify every time
Only ['count', 'distance', 'duration'] are used on the TravelSection; add type TravelMetricName to reflect this
I have not added unit utils for 'footprint' yet; but currently it is only used for ['count', 'distance', 'duration'] anyway
There are some places where we want an instance of colors accessible from outside a component, ie. not via useAppTheme
Makes this implementation match what the existing tests are expecting
Updates comment since it was out of date
gives a "You vs. Group" comparison for average daily carbon and energy
we do not need to lookup the label by language beacuse getFootprintGoals already handles this
This is necessary to be able to appropriately divide aggregate metrics back to per-user scale for "You vs. Group" comparison
customFootprint not needed anymore; replaced by emcommon footprint calculations aka CHEER

'mets' are not currently in use, but the data in 'metDataset.ts' is included in the base modes in emcommon, so we do have this available to add back in the future
confirmHelper
'json/label-options.json.sample' does not exist anymore so let's just test the default, if we passed a blank config

diaryHelper
Most of the functions that were tested here don't exist anymore and the tests can be removed. In fact, getDetectedModes is the only one left.
However there are functions that didn't have tests, add those

footprintHelper
All of these functions were removed and there is only one new one, getFootprintGoals

metricsHelper
The date format changed from numerical month/day to abbreviated month/day

imperialConfig
Extract the imperialConfig generation to a separate function so it can be accessed outside the hook.
Simpler and makes it easier to test.
Also let's test both metric and imperial, not just metric
if acc is not defined or is a number (as with distance,duration,count), do not add nUsers
We created this test a while ago intended as an example. We now have other component tests and don't need an example lying around
I added the StatusBar plugin so we can change the color to match the appbar color
@JGreenlee JGreenlee marked this pull request as ready for review September 19, 2024 20:22
Copy link

codecov bot commented Sep 19, 2024

Codecov Report

Attention: Patch coverage is 31.47208% with 270 lines in your changes missing coverage. Please review.

Project coverage is 28.83%. Comparing base (4596d0c) to head (9c2186b).
Report is 53 commits behind head on master.

Files with missing lines Patch % Lines
www/js/metrics/footprint/WeeklyFootprintCard.tsx 0.00% 40 Missing ⚠️
www/js/metrics/MetricsTab.tsx 0.00% 36 Missing ⚠️
www/js/metrics/footprint/FootprintSection.tsx 0.00% 33 Missing ⚠️
...w/js/metrics/footprint/FootprintComparisonCard.tsx 0.00% 24 Missing ⚠️
...ww/js/metrics/movement/WeeklyActiveMinutesCard.tsx 0.00% 23 Missing ⚠️
www/js/metrics/SumaryCard.tsx 0.00% 18 Missing ⚠️
www/js/metrics/movement/DailyActiveMinutesCard.tsx 0.00% 15 Missing ⚠️
www/js/metrics/MetricsScreen.tsx 0.00% 14 Missing ⚠️
www/js/survey/multilabel/MultiLabelButtonGroup.tsx 0.00% 7 Missing ⚠️
www/js/metrics/travel/TravelSection.tsx 0.00% 6 Missing ⚠️
... and 20 more
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1176      +/-   ##
==========================================
- Coverage   30.14%   28.83%   -1.32%     
==========================================
  Files         118      122       +4     
  Lines        5172     5012     -160     
  Branches     1110     1145      +35     
==========================================
- Hits         1559     1445     -114     
+ Misses       3611     3565      -46     
  Partials        2        2              
Flag Coverage Δ
unit 28.83% <31.47%> (-1.32%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines Coverage Δ
www/js/App.tsx 0.00% <ø> (ø)
www/js/Main.tsx 0.00% <ø> (ø)
www/js/appTheme.ts 100.00% <100.00%> (ø)
www/js/components/AlertBar.tsx 6.25% <ø> (ø)
www/js/components/BarChart.tsx 0.00% <ø> (ø)
www/js/components/Carousel.tsx 100.00% <100.00%> (ø)
www/js/components/Chart.tsx 0.00% <ø> (ø)
www/js/components/NavBar.tsx 83.33% <100.00%> (+4.76%) ⬆️
www/js/components/ToggleSwitch.tsx 0.00% <ø> (ø)
www/js/control/ProfileSettings.tsx 0.00% <ø> (ø)
... and 40 more

... and 1 file with indirect coverage changes

@JGreenlee
Copy link
Collaborator Author

JGreenlee commented Sep 20, 2024

Although the patch coverage is higher than the project coverage, the overall project coverage drops with this PR.
This is because I removed all the old carbon calculations, for most of which there were tests.

I added unit tests for new functions, withfootprintHelper.ts and metricsHelper.ts at or around 100% coverage.
The components do not have tests (nor did the old ones)

JGreenlee added a commit to JGreenlee/e-mission-translate that referenced this pull request Sep 20, 2024
Adds translations needed for e-mission/e-mission-phone#1176, plus other translations that I have noticed are missing
Copy link
Contributor

@shankari shankari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So glad that I just have a do a cursory review. At this point, my plan is to just merge, but I will flag any high-level issues to be addressed in a future cleanup.

The screenshots/design look great; do you have an example of what the survey screen looks like?

"e-mission-common": "github:JGreenlee/e-mission-common#semver:0.5.4",
"e-mission-common": "github:JGreenlee/e-mission-common#semver:0.6.1",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I should really review the e-mission-common changes at some point too. Maybe I can do that while generating the results for the paper.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe the easiest way to do that would be looking at the releases on e-mission-common and the "Full changelog"s that are generated there
https://github.com/JGreenlee/e-mission-common/releases

@@ -63,6 +63,13 @@ const EnketoModal = ({ surveyName, onResponseSaved, opts, ...rest }: Props) => {
useEffect(() => {
if (!rest.visible || !appConfig) return;
initSurvey();

// on dev builds, allow skipping survey with ESC
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so cool! ⏩ This will affect the demographics survey for sure, but won't it also affect the other trip-level surveys?!

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes it will affect them. This change was just focused on making testing in emulator easier, which was only cumbersome with the onboarding survey.
The trip-level surveys have a "dismiss" button, while the first time through the onboarding survey does not (by design)

I don't know that the check for __DEV__ was really needed, given that it is pretty hard to press ESC on a mobile app. But I did the principled thing and made sure it is only possible on dev builds

* @example getFormattedDate("2023-07-14T00:00:00-07:00") => "Jul 14, 2023"
* @example getFormattedDate("2023-07-14", "2023-07-15") => "Jul 14, 2023 - Jul 15, 2023"
*/
export function formatIso(isoStr1?: string, isoStr2?: string, opts?: Intl.DateTimeFormatOptions) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aren't there standard "humanize" functions for this? Are they not good enough?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are using Luxon's toLocaleString (which is like a wrapper around the standard Intl API's toLocaleString), which is doing most of the work here

The functions here exist for 2 reasons:
i) To format date ranges (e.g. Sep 17, 2024 – Sep 23, 2024) instead of only a single date (e.g. Sep 23, 2024)
ii) Shortcuts so I don't have to pass something like { month: 'short', day: 'numeric', year: 'numeric' } every time

www/js/util.ts Show resolved Hide resolved
Comment on lines -37 to +33
color: baseMode.color,
icon: baseMode.icon,
color: (confirmedModeForTrip || base_modes.BASE_MODES['UNPROCESSED']).color,
icon: (confirmedModeForTrip || base_modes.BASE_MODES['UNPROCESSED']).icon,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. Were we indeed only showing the basemode color before? Also, if this is actually the confirmed color instead of the confirmed mode object, you may want to use confirmedColorForTrip for clarity

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

color was solely a property of the base mode before. Rich modes could not have their own color separate from the base mode.
So previously it was using the color of the basemode of the confirmed mode. Now it is the color or the rich confirmed mode.

www/js/components/AlertBar.tsx Show resolved Hide resolved
www/js/metrics/metricsHelper.ts Show resolved Hide resolved
* @param metricName the metric that the values are for
* @returns the result of summing the values across all fields in the entry
*/
export function sumMetricEntry<T extends MetricName>(entry: MetricEntry<T>, metricName: T) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why doesn't this use aggMetricEntries? I guess because we don't need the aggregation and it is extra work to aggregate?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aggMetricEntries: aggregates/sums across entries
sumMetricEntry: sums within 1 entry
sumMetricEntries: does both

www/js/metrics/metricsHelper.ts Show resolved Hide resolved
@shankari shankari merged commit 6d69e15 into e-mission:master Sep 22, 2024
7 of 8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Tasks completed
Development

Successfully merging this pull request may close these issues.

2 participants