-
Notifications
You must be signed in to change notification settings - Fork 114
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
Label screen accessibility + custom labels + more profile migrations + upgrade to android 23 #1024
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is not yet complete, but is an idea of where the schedule table might eventually be
incorporating a patch from Jack with a solution to some of the issues with scheduled notifications, as well as cleaning up some of the style around the control-data-table (s) I converted earlier
found an error in the console alerting to the fact that each row should have a unique key, so I added keys one concern: the upcoming notifications keys end up being the dates - could those ever be repetitive?
took out the code that was a part of a different effort to prevent confusion, also removed commented out code form replacing elements
the "Upcoming Notifications" and "Dummy Notification in 5 Seconds statements were hard coded, I've added them as internationalized strings and updated the en.json file
I started to convert the data storage, data is fine in general-settings.js but is failing to make it through main-control.html to ControlDataTable.jsx
this is where I'm struggling with the parameterization of i18n keys
Added in code so the broken setting row is enabled, note the console.log statement demoing the different ways the info comes through
in Angular, string parameters need to be created as "'string'" - this should resolve some of the issues that I have been experiencing here Co-authored-by: Jack Greenlee <[email protected]>
after I learned how to pass strings, I was able to render the row as a list item, which will hopefully eventually be good for showing everything as a list, and can have icons built in Icons and passing through functionality are next
switching from plugin storage to scheduler storage Co-authored-by: Jack Greenlee <[email protected]>
after fix from Jack (2nd place to update where notifs come from) added in optional chain to reduce errors in table, took out diagnostic log statements, updated Logger.log to show num, time, and first notif
the setting-row process has been separated to More profile migrations #994
have a rough outline going of some setting rows that say and do the right thing! More complex actions, some text, and icons still need lots of work
I added a bunch of comments and whitespace to make it easier for myself to see what needs to be done -- I'll reformat once I have things working
most rows now migrated, still need to handle some with slightly unique functionality, styling with icons also needed!
all rows ugly but functional but need guidance on reminder time and demographics button
still missing some visual cues (toggle switches, expansion flippers)
now the button is what has action, not the entire row - there's a bug requiring an empty onPress in the list Item for the one in IconButton to work see issue callstack/react-native-paper#3898 and hopefully soon to be fix callstack/react-native-paper#3908
the two items that had switches are now interactive again, there is work to be done with getting the switches to have the proper starting value toggle switch: https://callstack.github.io/react-native-paper/docs/components/Switch/
converted the user data section to a list accordian - works great! Unfortunately relies right now on a lot of hard-coding
More profile migrations
There was one key that I eliminated because I could not find it used, it turns out the wrong key was being referenced so I restored the key and corrected the reference here
I had eliminated this key because it was not used, but in review we figured out where it was intended to be used and have restored it and updated the reference
- Support building individual platforms with the prod JS but dev native code - This will allow us to test the prod JS by connecting with a debugger (if needed) - Also allows quicker dev cycle by building only one platform at a time (note that for testing outside the devapp, we need to build the app every time) - Ensures that we don't need to get off the VPN just to build iOS - Ensures that we can build prod JS iOS version for archival and submission - Support building android with the prod JS and prod native code - To support building the aab to upload to the play store Testing done: - Used for building the NREL OpenPATH app
So that we use webpack for the javascript properly Testing done: - Used to build the NREL OpenPATH app
So that we can archive it to check out old functionality if needed
was missing the consent.permissions key somehow, adding it back here
Removing Stale i18n Keys
- `cordova-android` to 12 - `cordova-plugin-file` to 8.0.0 - Remove SDK version overrides and go with the cordova-android defaults Note that this will not work until e-mission/e-mission-data-collection#207 is merged #1016 (comment)
Everything should compile now. We can check in the sdk changes independently; the main goal is to start testing these changes before the release.
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.
🔧 ⬆️ 🚸 Upgrade android to API 33
🏷 Accessibility and Usability Improvements on Label Tab
🔧 ⬆️ 🚸 Upgrade android to API 33
We use a Babel plugin `angularjs-annotate` to fix our bad, implicitly annotated, Angular code. This way, it still works when bundled. But we only have Babel apply this to .js files. I moved a little bit of Angular code to .ts (the "Logger" service). I don't expect we will usually put Angular code in .ts files, so I'm just going to explicitly type it in the one place we do.
🚑 Fix Angular Annotations in logger.ts
The use of .options on these lines was always resulting in modeOptions and therefore the customMETs an CustomFootprint in being undefined. This was breaking and causing errors on the dashboard on staging.
🚑 🐛 Staging dashboard error fix
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Related PRs: