🏷️ Label UX: show labeled modes on map, update LabelOptions #1023

JGreenlee · 2023-08-29T20:01:47Z

This PR applies what was discussed in e-mission/e-mission-docs#952:

labelOptions will have a baseMode field that maps them onto a mode we recognize and have an icon+color for
Once a trip is labeled, the mode shown on the map will the labeled mode; no longer the sensed or inferred mode
Unlabeled trips will still show the sensed or inferred mode(s)
-- If the trip is unimodal, there is no need to show percentages

Also changed in labelOptions, the rename of co2PerMeter -> kgCo2PerKm, in accordance with e-mission/nrel-openpath-deploy-configs#30

In general, the code for the Label tab has been tidied, especially with regard to fixing typings.
Everything that Typescript and the VSCode linter picked up on has now been addressed.

This adds type definitions for the React hooks and makes some of the annoying typing errors go away. I don't know why the React package doesn't have types built-in, but this is how you get them.

With extensive work having been done on the new Label screen code, there are loose ends. I combed through all the files until the VSCode linter was happy :) This mostly meant removing unused imports/ unused code, but also fixing typings in some situations, or fixing a property name where I used the wrong one before. Another thing I fixed: some components were importing Text from 'react-native' when we want to be importing Text from 'react-native-paper' (it was barely noticeable, but some components were using a different font than others) A few specific details below: DiaryCard.tsx: -instead of accessibilityHidden, it should be aria-hidden (I think it used to be accessibilityHidden and the spec changed) StartEndLocations.tsx: --minWidth: 'fit-content' doesn't seem officially supported by RN. flexShrink: 0 is a suitable replacement TimestampBadge.tsx: --we're nesting <Text> components where they don't officially belong. However, it works - and Badge can't give us the desired result otherwise. So I chose to ignore this error for now --I was using numbers for fontWeight, but they should have been strings. This wasn't caught before because I was using a bare JS object instead of StyleSheet.create DateSelect.tsx: --DatePickerModal doesn't actually support the accessibilityLabel prop. However, looking through the source code revealed that RN Paper Dates does give much consideration to accessibility. So I think this is handled sufficiently, internally in the library. --Even though we are going to close the modal as soon as a date is picked (onChange), we still have to provide a callback for onConfirm (the save button) because it's a required prop. If they hit the save button without changing anything, we can just dismiss. AddedNotesList.tsx: --getFormattedDateAbbr actually takes an ISO string, not a unix timestamp. This was changed some time ago but not updated here (thanks typescript) EnketoModal.tsx: --EnketoModal should extend on the props that Modal takes, except it won't receive any children because it injects it own content based on what surveyName was passed in. So yes, it has all the props of Modal plus a few more, except we omit 'children'. MultiLabelButtonGroup.tsx: --modalVisibleFor is either a string or null. If a truthy string, it was being dynamically cast to true. But Typescript wants us to specifically give 'visible' as a boolean so we add `!= null`

We use `fetchUrlCached` on resources for which we make a network call. Local resources, however, do not need to be cached. For these, we can just use 'fetch' directly and parse as JSON with the fetch API.

The investigation in e-mission/nrel-openpath-deploy-configs#30 revealed that this field was mislabeled - it is actually kg per kilometer, not per meter. Fortunately, all the numbers are correct, so we can just give it a new name that is accurate and descriptive.

Before this, we only defined 'MotionTypes', but this was misleading - it contained the MotionTypes from the server but also included all the other modes. Let's keep MotionTypes consistent with the server and consider BaseModes something else - BaseModes can include MotionTypes, plus any other mode we support by default.

We use a field called 'percentages' to display the mode icon and percentage on the trip card above the map. However, this is an object containing more than just the percentage - it has mode, icon, color, and percentage. Soon, we plan to not even show the percentage in unimodal trips. So before that, let's rename this to something that is more descriptive.

We can call getLabelOptions only once and share it with everything under the Label tab component tree - this is more efficient than calling getLabelOptions in each component it is used.

- it should be an array of objects because there are multiple options - instead of 'key', the mode is identified by 'value' - add baseMode - met and met_equivalent are both optional (only one of the two will be defined)

I am extracting this out to a separate component since its logic is getting more complex as we show either the detected modes or the labeled mode on the map. This new component lists the detected or labeled modes at the top of the leaflet map on a trip card. If the trip has not been labeled, it will show "Detected:" with icon(s) for the mode(s) during the trip. If the trip has been labeled, it will show the labeled mode (which is in trip.userInput.MODE) and use the icon and color of that mode's `baseMode` (which is defined in labelOptions config). (e.g. 'free_shuttle' will show a purple bus icon if its base mode is "BUS")

When a trip is labeled, we need to know what icon and color to show above the map. To get this information, we will require each labelOption to have a baseMode, allowing us to map it onto a set of recognized modes ('BaseModes' in diaryHelper.ts) A couple new BaseModes were added, since the Material Design Icons library has icons some of the more niche modes that we probably didn't have icons for previously.

If the mode is user-defined, we will not find it in the labelOptions. In this situation, we should fallback to "OTHER" so that we show it in taupe (the color assigned for user-defined modes)

We have 2 ways by which we lookup a base mode - one is with the key and the other is when a trip has been labeled, we look up its `baseMode` from the labelOptions. Let's refactor to keep these in the same place in diaryHelper.ts and name them so it is clearer what they do.

We reworked ModeIndicator so that once a trip is labeled, it show the labeled mode icon and color. Now we need to have the map update the color of the trajectory to match the mode icon. services.js > confirmedPoints2Geojson is where the geojson is created and the color is specified there. If the trip is labeled, we will call getBaseModeOfLabeledTrip. For this, we need the labelOptions object, so we need to pass this down through the call stack from LabelTab, down to compositeTrip2Geojson, and finally to confirmedPoints2Geojson. Then we can get the labeled base mode, or if the trip isn't labeled, use the sensed_mode as we did before. To get the map to actually update in real time, we need to have the geojson recomputed in repopulateTimelineEntry, and we also need LeafletView to listen for changes to 'geojson'. Once that is all addressed, we can finally see the trajectory colors change in real time upon changing the labeled mode.

JGreenlee · 2023-08-29T20:02:35Z

TODO: after a trip is labeled, we should still be able to access the sensed/ inferred modes in the details screen

If there are too many modes to fit on one line, ModesIndicator can go onto two lines to fit them all. But if there are multiple rows we don't want a gap between them, so let's switch to 'columnGap' instead of 'gap' (which does both)

We were returning early if detectedModes was empty or the only detected mode was UNKNOWN. But once that trip is labeled, we won't be using detectedModes, and we should still show the mode that was labeled. So let's move these checks down to and else if condition, after we've already checked for a user label. If both if and else if conditions fail, then nothing will be rendered.

If we want to be able to switch back and forth between 'detected modes' and 'labeled mode', we need to approach the generation of geojsons in a different way. Precomputing and recomputing is not the best option because the map needs to change every time someone labels or toggles between 'detected'/'labeled'. We'll refactor the functions that did this in the Timeline service and rewrite new versions in timelineHelper. The trip geojson will be computed by useGeojsonForTrip (which caches previously computed geojsons so performance does not take a hit). Then we can call this where it is needed instead of precomputing and storing 'geojson' in the trip object.

The geojson object is no longer stored in the trip; we will access it in the 2 places it's used by using the new 'useGeojsonForTrip' function. On the details screen, if a trip has been labeled, we will be able to toggle between 'detected' and 'labeled' modes using SegmentedButtons (if set to 'detected', no 'labeledMode' will be passed to useGeojsonForTrip). If the trip is not labeled, we cannot toggle and just show "Detected Modes", aiming to match the style

sorted components into sub-folders: - StartEndLocations moved into js/diary/components - LabelDetailsScreen moved into js/diary/details (it will be split up into smaller components) - LabelListScreen moved into js/diary/list - DiaryButton moved out of js/diary into /js/components Now all that is at the root level is the main LabelTab component and 'helper' files

extracts parts of LabelDetailsScreen down into two new components: 1) TripDescriptives, which describes the overall trip distance, time, and modes; and 2) TripSectionsDetails, which gives a breakdown of each section in the trip.

The label details screen will be moved around; there is room there for multilabel buttons to be shown in one row so we will accept an optional boolean to make the buttons show inline

So that things make more sense with the "Labeled Mode" / "Detected Modes" toggle, the multilabel/ userinput button(s) will now show above the map. After the map, we'll show the section breakdown, and if there are multiple sections shown, we'll conclude with the overall trip descriptives. -Adding a rowGap makes the sections of this page feel less cramped together

if the toggle on the details screen is set to show the labeled mode, we want to be able to show that here instead of the sensed sections. If the trip is labeled, we treat it as unimodal regardless of what it was detected as. So if we are showing the labeled mode, we can construct a 1-item array and use the properties of the whole trip for distance and duration. And for the mode icon + color, we can call getBaseModeOfLabeledTrip, just as we have done elsewhere in the codebase. Then fill these in place of the detected 'section' properties. Some of the 'useDerivedProperties' variable names were changed here to make them more understandable to other maintainers in the context of the UI

Sticking with the term "descriptives" to avoid confusion with "details". (Since the entire screen is called "trip details", we don't want to have a smaller component also called "trip details" And since the "TripDescriptives" component (describing the whole trip) is only shown if there are multiple sections showing (to avoid redundancy), it's clearer if we call it "OverallTripDescriptives".

-fix padding since this adds another container

-add padding to multilabel/userinput button row -remove hardcoded font sizes; use text variants instead -unify info between section descriptives and overall trip descriptives -unify styles

e-mission#30 was merged, but there are a few things missing to get dynamic label options working fully as expected: - change key -> value -- the phone expects each option to have a field by the name of 'value'. The old trip_confirm_options used 'value'. I think I was using 'key' in the new format because it is more concise, but we still need to support both this format and the trip_confirm_options format, and it is not worth going back and changing all the places in the phone code where 'value' is used. -add baseMode for each of the MODE options (to work with e-mission/e-mission-phone#1023) -add missing modes: "bikeshare" and "not_a_trip". Evidently, these were left off in the first PR. I think it may have been based on an older version so I checked all the options against the newest trip_confirm_options. -capitalize English text for 'E-bike' and make it appear before 'bike' - this is what the latest trip_confirm_options has

JGreenlee · 2023-08-30T22:23:40Z

TODO: after a trip is labeled, we should still be able to access the sensed/ inferred modes in the details screen

I have made much progress on this. I reworked the label details screen to include a toggle between "Labeled Mode" and "Detected Modes" on trips that have the mode labeled. Unlabeled trips only show "Detected Modes".

JGreenlee · 2023-08-30T22:27:32Z

For this to make sense, I reordered the Details screen altogether. See it in this demo:

To me, this was the most logical way to frame it and make sense of the toggling. Open to feedback.

e-mission#1014 (comment)

as described in e-mission/nrel-openpath-deploy-configs#32, we'll stick with 'value' as the field by which to identify options, not 'key'.

e-mission#1014 (comment)

I am cherry-picking this component ToggleSwitch from https://github.com/e-mission/e-mission-phone/blob/c4684782bd19fbc6d12e8de9194d6a395a732f07/www/js/components/ToggleSwitch.tsx It is from another PR that is being worked on concurrently - when both PRs are merged they should be even. Also adding the colors that ToggleSwitch uses.

Inferred labels used to be a bright yellow with white text on them. Since white text on yellow is hard to read, the button color was shifted to a deeper orange to achieve better legibility / color contrast ratio. However, we received some user feedback that the deep orange felt intimidating. To the user, this seemed like some kind of warning or error. They asked us to bring back the yellow color. So I reworked DiaryButton to work with dark text and accept a custom border color. Now, inferred labels show on a pale yellow button with dark text, framed by a yellow border.

These were used by older versions of the app and have been made obsolete by the new label screen.

This is not necessary; the string "other" never even gets stored as a value because we ask users to fill in their own value. e-mission#1014 (comment)

Since MOPED was included in e-mission/nrel-openpath-deploy-configs#30, the config will be looking for an icon + color for MOPED (at least on dev-emulator configs for now). The MDI library actually has an icon for 'moped', and we'll use green since a moped is a) a derivative of a bicycle and b) fairly emission-friendly Note that this doesn't make MOPED show up on all configurations (or even the default configurations) - only when the labelOptions points to MOPED as a baseMode for one of its mode options

JGreenlee · 2023-08-31T05:13:18Z

Ready for review and should be merged at the same time as e-mission/nrel-openpath-deploy-configs#32.

shankari

I am done with most of the review, except for ~ 10 of the more complex files.
Will finish review tomorrow morning and get this onto staging.

shankari · 2023-09-05T01:24:20Z

www/js/diary/LabelTab.tsx

@@ -11,15 +11,15 @@ import { angularize, getAngularService } from "../angular-react-helper";
 import useAppConfig from "../useAppConfig";
 import { useTranslation } from "react-i18next";
 import { invalidateMaps } from "../components/LeafletView";
-import Bottleneck from "bottleneck";


we don't need bottleneck any more because of caching?

Bottleneck is still used in addressNamesHelper.ts but unused in LabelTab.tsx

shankari · 2023-09-05T01:25:44Z

www/js/diary/LabelTab.tsx

@@ -54,6 +55,7 @@ const LabelTab = () => {
    const surveyOpt = SurveyOptions[surveyOptKey];
    setSurveyOpt(surveyOpt);
    showPlaces = appConfig.survey_info?.buttons?.['place-notes'];
+    getLabelOptions().then((labelOptions) => setLabelOptions(labelOptions));


ah I see that we are caching this now. So can we resolve #1014 (comment) ?

shankari · 2023-09-05T05:05:56Z

www/js/diary/timelineHelper.ts

+  if (!trip.sections) {
+    // this is a unimodal trip so we put all the locations in one section
+    sectionsPoints = [locationList];
+  } else {
+    // this is a multimodal trip so we sort the locations into sections by timestamp
+    sectionsPoints = trip.sections.map((s) =>
+      trip.locations.filter((l) =>
+        l.ts >= s.start_ts && l.ts <= s.end_ts
+      )
+    );


I'm confused. I see from the demo that when there is a user label, we essentially display the trip as unimodal.
However, in that case, we must be setting the trip.sections to [] somewhere so that we don't just fall through and execute the multi-modal trip use case. But I don't see where that is.

trip.sections is never modified because we want to treat the trip object as immutable.

If a trip has been labeled with a mode, (ie trip.userInput.MODE.value exists), we keep trip.sections as it is and just don't use it.
That discrimination happens a few places including ModesIndicator and TripSectionsDescriptives

shankari · 2023-09-05T05:06:51Z

www/js/diary/services.js

@@ -1,7 +1,7 @@
 'use strict';

 import angular from 'angular';
-import { getFormattedTimeRange, motionTypeOf } from './diaryHelper';
+import { getBaseModeByKey, getBaseModeOfLabeledTrip } from './diaryHelper';


nit: presumably you don't need this any more?

Yeah, none of these external functions are needed anymore

There are only a few functions left in this file though - basically just readAllCompositeTrips and readUnprocessedTrips and the smaller functions they depend on.

I think we can incorporate those into timelineHelper.ts on the next pass and services.js will no longer be needed

shankari · 2023-09-05T05:15:22Z

www/js/survey/enketo/AddedNotesList.tsx

@@ -33,7 +35,7 @@ const AddedNotesList = ({ timelineEntry, additionEntries }: Props) => {
    if (isMultiDay(beginTs, stopTs)) {
      const beginTsZoned = moment.parseZone(beginTs*1000).tz(timezone);
      const stopTsZoned = moment.parseZone(stopTs*1000).tz(timezone);
-      d = getFormattedDateAbbr(beginTsZoned.unix(), stopTsZoned.unix());
+      d = getFormattedDateAbbr(beginTsZoned.toISOString(), stopTsZoned.toISOString());


Note to self: this was changed in 7eedb92 because

"--getFormattedDateAbbr actually takes an ISO string, not a unix timestamp. This was changed some time ago but not updated here (thanks typescript)"

shankari · 2023-09-05T05:18:32Z

www/js/diary/list/DateSelect.tsx

@@ -71,20 +71,20 @@ const DateSelect = ({ tsRange, loadSpecificWeekFn }) => {
      <Text>{dateRangeEnd}</Text>
    </NavBarButton>
    <DatePickerModal locale={i18next.resolvedLanguage || 'en'}
-      accessibilityLabel="Select a date"


Note to self, this is also handled in the cleanup commit
eedb92ce96580238ca4444328560275ed4f84b1

due to

--DatePickerModal doesn't actually support the accessibilityLabel prop. However, looking through the source code revealed that RN Paper Dates does give much consideration to accessibility. So I think this is handled sufficiently, internally in the library.

shankari · 2023-09-05T05:23:03Z

www/js/diary/diaryHelper.ts

  red: '#b9003d',         // oklch(50% 0.37 15)     // car
  orange: '#b25200',      // oklch(55% 0.37 50)     // air, hsr
-  green: '#007e46',       // oklch(52% 0.37 155)    // bike
+  green: '#007e46',       // oklch(52% 0.37 155)    // bike, e-biek, moped


nit: e-bike

shankari

This is the full review. I don't see anything that cannot be cleaned up later, so I am pulling this and getting it to staging.

shankari · 2023-09-05T15:22:21Z

www/js/diary/diaryHelper.ts

+  // END MotionTypes
+  CAR: { name: "CAR", icon: "car", color: modeColors.red },
+  E_CAR: { name: "E_CAR", icon: "car-electric", color: modeColors.pink },
+  E_BIKE: { name: "E_BIKE", icon: "bicycle-electric", color: modeColors.green },
+  E_SCOOTER: { name: "E_SCOOTER", icon: "scooter-electric", color: modeColors.periwinkle },
+  MOPED: { name: "MOPED", icon: "moped", color: modeColors.green },
+  TAXI: { name: "TAXI", icon: "taxi", color: modeColors.red },


I think we should clarify that this is "default base modes" given that we now support custom labels and the base mode might be in the custom label map.

I thought we decided that if a custom mode is defined in labelOptions, they would have to map it to one of these pre-defined base modes?

shankari · 2023-09-05T16:47:01Z

www/js/diary/details/OverallTripDescriptives.tsx

should this be in components? Do we plan to reuse it anywhere else?

I can't think of anywhere we would currently show details about a trip other than the Label Tab Details screen

JGreenlee added 13 commits August 24, 2023 23:40

add @types/react to devDependencies

cc87d0c

This adds type definitions for the React hooks and makes some of the annoying typing errors go away. I don't know why the React package doesn't have types built-in, but this is how you get them.

fetch local labelOptions without caching

3f892a0

We use `fetchUrlCached` on resources for which we make a network call. Local resources, however, do not need to be cached. For these, we can just use 'fetch' directly and parse as JSON with the fetch API.

include labelOptions in LabelTabContext

c3afa3b

We can call getLabelOptions only once and share it with everything under the Label tab component tree - this is more efficient than calling getLabelOptions in each component it is used.

fix types for LabelOptions

b6d7bfb

- it should be an array of objects because there are multiple options - instead of 'key', the mode is identified by 'value' - add baseMode - met and met_equivalent are both optional (only one of the two will be defined)

fix mode indicator for user-defined modes

79a9ee6

If the mode is user-defined, we will not find it in the labelOptions. In this situation, we should fallback to "OTHER" so that we show it in taupe (the color assigned for user-defined modes)

JGreenlee added 13 commits August 29, 2023 16:06

allow ModesIndicator to wrap if many modes

a426834

If there are too many modes to fit on one line, ModesIndicator can go onto two lines to fit them all. But if there are multiple rows we don't want a gap between them, so let's switch to 'columnGap' instead of 'gap' (which does both)

refactor LabelDetailsScreen into smaller components

38e21ca

extracts parts of LabelDetailsScreen down into two new components: 1) TripDescriptives, which describes the overall trip distance, time, and modes; and 2) TripSectionsDetails, which gives a breakdown of each section in the trip.

MultiLabelButtonGroup: support inline buttons

95d875e

The label details screen will be moved around; there is room there for multilabel buttons to be shown in one row so we will accept an optional boolean to make the buttons show inline

add "Overall" title to OverallTripDescriptives

d8ffec5

-fix padding since this adds another container

adjust styles, text on label details screen

a2adde7

-add padding to multilabel/userinput button row -remove hardcoded font sizes; use text variants instead -unify info between section descriptives and overall trip descriptives -unify styles

trip details descriptives: labeled text under icon

3c9b6dc

JGreenlee mentioned this pull request Aug 30, 2023

Add baseModes, fix 'value', add missing modes e-mission/nrel-openpath-deploy-configs#32

Merged

JGreenlee mentioned this pull request Aug 30, 2023

🏷 Accessibility and Usability Improvements on Label Tab #1014

Merged

JGreenlee added 9 commits August 30, 2023 19:55

check mode_studied instead of program_or_study

7dc227c

e-mission#1014 (comment)

labelOptions: use 'value' instead of 'key'

1e67c88

as described in e-mission/nrel-openpath-deploy-configs#32, we'll stick with 'value' as the field by which to identify options, not 'key'.

> check mode_studied instead of program_or_study

32274bc

e-mission#1014 (comment)

i18n cleanup

7fed43e

remove unused HTML templates

2639359

These were used by older versions of the app and have been made obsolete by the new label screen.

skip checking inferred.value != "other"

de75f4b

This is not necessary; the string "other" never even gets stored as a value because we ask users to fill in their own value. e-mission#1014 (comment)

JGreenlee marked this pull request as ready for review August 31, 2023 05:10

shankari requested changes Sep 5, 2023

View reviewed changes

shankari changed the base branch from aria_and_fall_2023_rewrite to label_dashboard_profile_sept_2023 September 5, 2023 16:55

shankari merged commit d6637a6 into e-mission:label_dashboard_profile_sept_2023 Sep 5, 2023

JGreenlee mentioned this pull request Feb 13, 2024

UX Considerations for display of mode percentages e-mission/e-mission-docs#952

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

🏷️ Label UX: show labeled modes on map, update LabelOptions #1023

🏷️ Label UX: show labeled modes on map, update LabelOptions #1023

JGreenlee commented Aug 29, 2023

JGreenlee commented Aug 29, 2023

JGreenlee commented Aug 30, 2023

JGreenlee commented Aug 30, 2023

JGreenlee commented Aug 31, 2023 •

edited

Loading

shankari left a comment

shankari Sep 5, 2023

JGreenlee Sep 5, 2023

shankari Sep 5, 2023

shankari Sep 5, 2023

JGreenlee Sep 5, 2023

shankari Sep 5, 2023

JGreenlee Sep 5, 2023

shankari Sep 5, 2023

shankari Sep 5, 2023

shankari Sep 5, 2023

shankari left a comment

shankari Sep 5, 2023

JGreenlee Sep 5, 2023 •

edited

Loading

shankari Sep 5, 2023

JGreenlee Sep 5, 2023 •

edited

Loading

🏷️ Label UX: show labeled modes on map, update LabelOptions #1023

🏷️ Label UX: show labeled modes on map, update LabelOptions #1023

Conversation

JGreenlee commented Aug 29, 2023

JGreenlee commented Aug 29, 2023

JGreenlee commented Aug 30, 2023

JGreenlee commented Aug 30, 2023

JGreenlee commented Aug 31, 2023 • edited Loading

shankari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

shankari left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JGreenlee Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JGreenlee Sep 5, 2023 • edited Loading

Choose a reason for hiding this comment

JGreenlee commented Aug 31, 2023 •

edited

Loading

JGreenlee Sep 5, 2023 •

edited

Loading

JGreenlee Sep 5, 2023 •

edited

Loading