🏷 Accessibility and Usability Improvements on Label Tab #1014
Conversation
Assistive technologies should recognize this as a graphic called "Map" and not read anything else about it.
Icons that aren't clickable should be marked as presentational / not interactive and not focusable so that screen readers don't pick up on them. Up to this point we've been using IconButton for everything, clickable or not. Let's make a new component Icon, which is a wrapper around IconButton that marks it as not interactive and disables the padding so it's just the plain icon. From now on, clickable icons will use <IconButton> and plain, presentational icons will use <Icon>.
This ensures that when the details screen is opened, the focus is moved to the new screen rather than staying on the previous main screen. We don't want screenreaders to stay focused on elements that are no longer visible.
In TripCard, the left and right panels were reordered. They still appear on the left and right respectively, but now the right panel is ordered before the left panel. This is so that screenreaders read the right panel (which has the basic trip info like date, duration, locations) before the left panel. This should be clearer to blind/visually impaired users who rely on audio cues. Visually, the panels are still rendered the same as before because the flexDirection of their parent is now 'row-reverse'
The Enketo services, written in Angular, will be rewritten and consolidated into React components and Typescript functions. EnketoModal will be the UI component that displays embedded surveys in a popup modal. It accepts the same arguments and options as the EnketoSurvey service.
add the validateAndSave function to EnketoModal, along with all the logic needed to support it. Functions have been rewritten from enketo/service.js and enketo/launch.js. I did not rewrite enketo/answer.js, so the EnketoSurveyAnswer service is still used in enketoHelper.ts. Nothing has dramatically changed here - I just rewrote the same logic in Typescript. One thing that has changed is the way that validation errors appear. If something is wrong in the survey response, we used to show an IonicPopup describing the issue. IonicPopup has a lower z-index than the React Native Modal. Rather than try to hack the z-index and compete for the foreground, I opted to show these popups using `window.alert` instead. I tested it on both Android and iOS, and found that the window alerts look like the native popups on their respective OS. It actually provide a much nicer UX for errors and I will move the Logger over to using this as well.
I am leaving the original Angular service in here because other files still use it. Both can remain in this file until nothing uses the Angular service anymore.
This handy function can be used for any URL resource that we'd like to be cached to localStorage. For right now we'll just use this for any surveys loaded by EnketoModal. I decided to put this in a new file called commHelper.ts. This is a new function, but the existing Angular CommHelper service will be rewritten at some point and those new functions can go in this file too.
I think we are done learning how to use React, with plenty of working examples in the codebase
We'll capture any other props with ...rest and apply these to the underlying IconButton
- fix Icon colors (the correct prop to use is iconColor, not color) - ensure display times always fit on 1 line - make top bar title smaller (it was huge) - wrap the contents in a SafeAreaView so we don't have to guess about padding offsets for statusbar / notch
|
This is how I rewrote the multilabel popups:
I think this is a big improvement for accessibility as well as to the UX of the app in general |
|
The Enketo code has been refactored into React and is fully functional for time-use. I have left the old Enketo code there because it is still being used for the other surveys.
I didn't plan to fully rewrite the Enketo code until later, but it was such a barrier to accessibility that I wanted to address it while I'm focusing on these accessibility tasks. |
Instead of using Ionic popups and separate templates for the mode and purpose select menus, we can utilize a Dialog containing Radio buttons using components from React Native Paper. This is much better in terms of accessibility - the Ionic popups were not able to be navigated to by keyboard / screen reader controls, while the Dialog gains focus when opened because it is wrapped in a Modal. The UX for inputting 'other' has been changed as well. Instead of having 'other' entry be a separate popup, I incorporated it into the same Dialog. When the "Other" radio button is chosen, a text entry and Save button will appear right below it allowing the user to specify their custom label.
The button to dismiss the Enketo Modal has been incorporated into the Enketo header so it doesn't take up a whole extra row. The accessibility roles / visibility have been tweaked for both the dismiss and save buttons so that they are exposed correctly to the accessibility tree as clickable buttons without extra nesting.
SurveyOptions is just an object with some definitions for the MULTILABEL vs ENKETO configurations. We don't need an Angular service for this; let's just make it an exported const object and import it where needed. Also with type safety! In TripCard, I noticed that we check the config and set surveyOpt as state. We already do this in LabelTab and can just use the surveyOpt from there via useContext. This must have been there from before we set up the LabelTabContext to share state. No reason to do this in both places. The directive 'linkedsurvey' is not used anymore so it is being removed.
Typically to receive clicks in React Native you need a Touchable or Pressable element, or something that composes one, and then use the onPress prop as a handler. But for Enketo, (web-based/HTML) we aren't working with React Native code; these are HTML elements. So I thought I had to wrap buttons in Pressables to receive any click events. It turns out that for HTML <button>s, we can use onClick instead of onPress and it works the same. So we don't need those extra <Pressable>s, and the <button> already gives itself an aria role of button. Add a little bit of styling to make the <button>s behave the same as the <Pressable>s did, and we're set. While here, I cleaned up the HTML and added a comment describing the template as well.
We extracted SurveyOptions and no longer needed the 'emission.survey' module - but it was listed as a dependency in one place, here. So 'emission.survey' no longer exists, but we do need to include the dependencies it had here in diary.js
Instead of the Angular services, the UserInputButton will now launch its survey using the EnketoModal, paralleling the way AddNoteButton works
if the survey does not have enough content on a page to fill the entire height of the screen, we should have it grow to fill the remaining space. This way, the footer is always at the bottom. Adding flex: 1 to everything in the modal up until the enketo-plugin realizes this behavior
We can launch surveys from this component when we edit previous survey responses, so we should use EnketoModal here too instead of the Angualar services.
When deleting an activity, we have an "Are you sure?" confirmation popup which was rendered using $ionicPopup. This has been converted to a Dialog in a React Native Modal.
When embedding React components in angular we used the `angularize` function and `propTypes`. But with the whole label screen rewritten in React, we don't have to do this anymore and can remove these and include Typescript typings instead.
|
This was, I believe, the last popup launchable from the Label screen that was using Ionic. 
So with this, I would consider the Label screen UI fully in React! |
DiaryButton was the first component written in React and it was a little bit off in terms of appearance. We can slim down the padding so more text can fit, and make the icon, if present, spaced evenly from the text. We don't need to pass the text as a separate prop; we can accept the content text as the child and embed it that way. Let's also type the Props as an extension of ButtonProps, since this component is basically a wrapper around the RN Paper <Button>.
I had modified mls so it reads a few things from the dynamic config. But it should be accessed as `mls.ui_config`, not `ui_config`.
Testing the dynamic label options revealed that the translations were not being filled in. This was because of a mistake here - we should loop through each opt in each of MODE, PURPOSE, and/or REPLACED_MODE, and fill them in one by one.
In the devapp or on dev builds, we have the __DEV__ global set true. We can utilize this to simplify the dev process of testing different configs. Instead of manually changing the downloadURL in dynamic_config.js to localhost:9090 when we want to test other configs, we can have dynamic_config.js automatically check localhost:9090 for locally served configs. If it doesn't find a local config, it fallbacks to downloading from github. On production, where __DEV__ is false, it will not check for local configs and will go straight to github (same behavior as before). In the process, I refactored this to use `fetch` instead of Angular's `$http` because it will make it easier to rewrite this file later.
I implemented this here since I was testing with local configs to support e-mission/e-mission-docs#945. |
JGreenlee
changed the base branch from
framework_migration_prep
to
aria_and_fall_2023_rewrite
this is handled by addressNamesHelper now; this function is not needed
functions kept as-is; except some syntax modernized With all of its functions now in diaryHelper.ts, the DiaryHelper factory can be removed altogether
The composite and confirmed trip/place objects we receive from the server don't include all of the strings we actually show in the UI. We do some formatting to these before showing these as 'display' props. Instead of 'populating' trip/place objects by mutating these additional properties into them when they are loaded, let's make a memoized hook that extracts the derived properties for a given timelineEntry. Then we can pull from these downstream where they're used. And we won't have to worry about 'populating' trips/place before they are used for display - the information contained in the trip/place objects should now be almost exactly the same as what is on the server. (Eventually unifying the type definitions to match 100% is a long-term goal)
and add comment describing what it does
All of the diary cards (TripCard, PlaceCard, UntrackedTimeCard), as well as the LabelDetailsScreen, all show the names of some locations, with only a few differences between. A bunch of code was repeated across each of these files, so we can refactor this into a reuasable component that we can call StartEndLocations.
For addresses that haven't been loaded yet, we can give a default value of an empty string instead of undefined. This allows us to distinguish downstream between a value that hasn't been loaded yet and a value that is not set.
e-mission/e-mission-docs#953 Instead of a greenish-grey for draft trips, we will now use a grey with a slight tint of blue.
In our config file, we may reference external resources by URL in the form of Enketo surveys or a 'label options' JSON file. To optimize this, we should have these be fetched + cached asynchronously when the config is first loaded. This way we won't have to wait for them to load when they are needed.
With the goal of eventual type safety with regard to composite trip / confirmed place objects, I have been tracking down which properties are actually present on the server and which ones are computed on the phone. Eventually, it would be great to have our type definitions for the Composite/Confirmed objects line up 1:1 between our JS and Python code. However, there is still some work to do before we can apply these type definitions anywhere (the "populated trip" properties still need to be reworked)
The label within the <Button> no longer needs display: flex if we add verticalAlign: 'middle'. This will prevent the inner container from growing with its content, allowing the text truncation to work properly. Adjust margin too - vertical margin is not needed and horizontal margin can be slightly more so text doesn't look cramped.
In dev environments (where __DEV__ == true), we will check localhost:9090 for locally hosted configs. if we are not serving at that moment, we should just skip. The fetch throws an error if localhost:9090 is not active - we should catch this, skip the locally hosted config and check fetch from github instead.
| @@ -2,6 +2,16 @@ | |||
| /* if we don't contain them here, they will leak into the rest of the app */ | |||
| .enketo-plugin { | |||
| @import 'enketo-core/src/sass/formhub/formhub.scss'; | |||
| flex: 1; | |||
There was a problem hiding this comment.
This is fine for this release, which I really don't want to make other changes to, but let's test out whether turning off pagination deals with autoscrolling properly and then turn off pagination by default. I think that is what we decided.
I think we will still need this - what if there is only one "yes/no" question - but we might be able to remove it then.
There was a problem hiding this comment.
Pagination being on or off isn't something we control from the phone code; it's actually an option in the forms and can be enabled/disabled from KoboToolbox or the Excel file or the XML file. When I brought up the topic, I was just suggesting that we change the built-in demographic survey to not use pagination.
But if someone else wants to supply their own surveys that do have pagination turned on, they can do that as much as they please.
The mock Time-Use survey we have been using does not use pagination and the scrolling has been fine.
There was a problem hiding this comment.
Right, I was suggesting that we turn pagination off for the default survey.
I know that the mock time use is not paginated, but it is also not very long.
I want to try it with a long survey, like the demographics, and see if the experience is smooth - e.g. does it automatically scroll down when a question is complete, and whether that feels weird or not?
| * @returns Promise of the fetched response (as text) or cached text from local storage | ||
| */ | ||
| export async function fetchUrlCached(url) { | ||
| const stored = localStorage.getItem(url); |
There was a problem hiding this comment.
Future: I think you can theoretically reuse this to cache the nomimatim reverse-lookup as well although the key length may be too long and it is not that important going forward anyway.
There was a problem hiding this comment.
The nominatim lookup is also cached using localstorage, but in a different way so that we can subscribe to its updates.
(for the key, I used the actual coordinates)
Both implementations are temporary anyway because localstorage is not a good long-term solution and we need to find an alternative eventually (it depends on what we are allowed to use)
| <View style={[cardStyles.cardContent, {marginVertical: 12}]}> | ||
| <View>{/* date and distance */} | ||
| <View style={[cardStyles.cardContent, {marginVertical: 12}]} focusable={true} | ||
| accessibilityLabel={`Untracked time from ${displayStartTime} to ${displayEndTime}`}> |
There was a problem hiding this comment.
Do these also need to be internationalized? I assume so!? Because if you are a spanish speaking user using a screenreader, presumably you want to hear it read out in Spanish. Unless the screenreader does some kind of translation by itself?
There was a problem hiding this comment.
I was wondering that, but figured that English labels were at least a good starting place.
Let's ask UW
| <View style={[cardStyles.cardContent, {flexDirection: 'row'}]}> | ||
| <IconButton icon='dots-horizontal' size={28} | ||
| <View style={[cardStyles.cardContent, {flexDirection: 'row-reverse'}]} | ||
| accessibilityLabel={`Trip from ${displayStartTime} to ${displayEndTime}`}> |
| {surveyOpt?.elementTag == 'multilabel' && | ||
| <MultilabelButtonGroup trip={trip} />} | ||
| {surveyOpt?.elementTag == 'enketo-trip-button' | ||
| && <UserInputButton timelineEntry={trip} />} |
There was a problem hiding this comment.
It seems like it would be useful to have a component that is the equivalent of linkedsurvey that we could use in any location. Or is the thought that it was just an artifact of the diary/label dichotomy and we will always use the trip component with the labels even if we want to label trips elsewhere? Future
There was a problem hiding this comment.
My thinking was that it's only 4 lines of code and it's only used in 2 places (TripCard and LabelDetailsScreen)
Extracting this to a separate component would save 6 lines of code but add ~15-25 in a new file due to boilerplate
| let sectionPcts = sortedKeys.map(function (mode) { | ||
| const fract = dists[mode] / totalDist; | ||
| return { | ||
| mode: mode, | ||
| icon: motionTypeOf(mode)?.icon, | ||
| color: motionTypeOf(mode)?.color || 'black', | ||
| pct: Math.round(fract * 100) || '<1' // if rounds to 0%, show <1% | ||
| }; | ||
| }); |
There was a problem hiding this comment.
can't you just use inferred/cleaned section summary here?
Next release
There was a problem hiding this comment.
I am not sure what is meant by "inferred/cleaned section summary". Can you clarify?
There was a problem hiding this comment.
As part of the changes prepping for moving "count every trip" into production, we added cleaned_summary and inferred_summary objects to the confirmed, and thus, the composite trips.
From emission/core/wrapper/confirmedtrip.py.
20 "inferred_section_summary": ecwb.WrapperBase.Access.WORM,
21 "cleaned_section_summary": ecwb.WrapperBase.Access.WORM,These contains the percentages per mode of the distance, duration, count, ...
It seems like you can use them directly instead of recomputing
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)
e-mission#1014 (comment) Confirmed and composite trips have section summaries that are computed on the server. We can use these here instead of using the raw 'sections'. Let's also add type definitions for the section summaries.
(this PR is targeted for after the current release, but since that keeps getting postponed I have been spending significant time working ahead, and it's time to check in a draft)
The accessibility issues that I am prioritizing now are largely to do with how each of the UI screens are structured and defined semantically. Particularly, I am paying mind to the accessibility tree being exposed by the app at any given moment, and how this will be received by assistive technologies.
There are plenty of smaller accessibility tasks - text that can be made more legible, click areas that can be made bigger, etc. But I am not going to worry about too many small things at this moment because we can easily address them down the road.
I found that the least accessible aspects of the Label screen, and the main interaction flow of the app in general, are to do with Ionic modals, popups, and popovers.
I've found that the React code we have written thus far seems decently intelligible and traversible to screen readers. For example, the React Native Modal does an adequate job of taking focus to the foreground when a modal is opened, while the Ionic popups left the focus in the background. This is good news for the migration; it means our new dialogs, multi-selects, and datepickers will already be more accessible than their old Angular counterparts.
Nonetheless, I noticed a lot of problems related to aria roles and labels and left fairly detailed comments in my commits. The goal there was mainly to clean up the accessibility tree and not have it cluttered by presentational junk.
Certainly the worst things for accessibility seemed to be the Enketo survey popups and multilabel popovers. I went ahead and began the work of rewriting them in React and using React Native Modals. I think the bulk of that work is done.