Profile changes to unblock routing #1029
Conversation
for react routing, we need to convert this page and the sync settings page maintain access for now by routing to a React page with angular routing using AlertBars for the error messages first draft: scrolling the FlashList is broken and the data doesn't load unless you manually tap refresh
Abby-Wheelis
changed the base branch from
master
to
label_dashboard_profile_sept_2023
…023' into unblock-routing
instead of making dialogStyle a parameter, export and import the stylesheet where it is used, to maintain consistency and limit parameters Needed something similar to some of the dialogs in ProfileSettings for the SyncLogs -- so create a component `ActionMenu` to show the list of options, replacing the two hard-coded modals (carbon and force state) with an instance of ActionMenu
so each has it's own title, add a title parameter
the other page that needs conversion is the one that shows sensed data, so convert it as well Resolved scroll issue by getting out of Angular -- using a full screen modal helps us accomplish this
we no longer rely on any of the angular code that controlled the log pages, so we can remove the files and the places they were imported
previously, was not loading more when you hit the end of the list, now the list loads more when you reach the end
load the log pages when their visibility changes, this seems cleaner than tying it to the config, since this is unrelated to the config
I was getting a mysterious "unrecognized selector" error and came to the conclusion that it was because loadStats.currentStart was null on first load, by setting a fallback, the error is resolved
these had been commented out, and by searching the codebase I came to the conclusion that they were for communicating with angular/ionic, which we no longer use unlike other broadcasts, this is not received by other parts of our code, to trigger some action, so we don't need it
|
Update to this PR -- the log and sensed pages have been converted, and are shown below. ^I just noticed the styling on these doesn't match, so I'll fix that now There are lots of parts that are not internationalized (title of the page, error messages), @shankari do we want to start internationalizing in dev zone while we're here, or leave it in English? |
I was using react-native Text when I wanted react-native-paper Text
|
@Abby-Wheelis I think that it depends on how much effort it would be. At a high level, any OpenPATH developer is going to have to know English since all our developer documentation is in English. However, the developer zone is not only for developers, we sometimes ask participants to use it for debugging (e.g. are you in the "waiting for trip start" or the "start" state?) So I guess my argument is that it would be good to translate if it is not too much effort. But if it is going to double the number of strings or something, we have higher priorities. |
Also, the messages are different - the one on the left shows the actual logs, the one on the right is apparently only showing state transitions??! |
I'd say it's ~10 or fewer strings, so worth it! We've updated a bunch recently so I also need to check in with the Lao team again soon (using google translate, and can't double check like I can with Spanish). |
The messages being different is ok, I think, since it's two different pages, which is also what we have in production now. Left page is for "check log" and right is for "check sensed". State transitions is one of three options for sensed data, Locations and Motion Types being the others, which you can change with the hamburger menu. -- Locations and Motion Types are blank lists when I check them here, and on my phone that's on production, so I need to keep looking into if this is what's expected. |
I think that bitrotted sometime in the distant past. They were supposed to show the list of locations and the list of motion activity objects (read from the usercache) to help debugging. I am fine with removing those - I don't remember the last time that I used them. |
Adding translations for the user-visible strings to ensure users can access the dev zone, the logs will be in English, but with headers and error messages in selected language participating in debugging will be easier
we don't need these show sensed or show logs anymore!
these two options are not supported, so we should not present them to the user -- removing them leaves us with one option, so no need for an options menu This reduces the complexity of the sensed data page significantly note about removing options: e-mission#1029 (comment)
|
I've added the i18n (only ended up being 4 strings) and removed the bit-rotted options on the sensed page. Translations for our other two languages here: Dev zone keys |
create a React version of ControlCollectionHelper and ControlSync Helper. These hold the logic for interfacing with the plugins, as well as UI elements for editing menus and various alerts / messages The place this is being stored has also changed, we no longer need to import from the other repos These helpers will eliminate the last parts of React routing, and allow us to no longer rely on the broadcast of "control update complete" to ensure the UI stays up to date
medium accuracy switch was broken, now can be toggled smoothly
no longer needed, as refreshCollect settings is called after toggling accuracy, toggling tracking, and when the settings editor is closed
no longer rely on settingsScope to catch recomputeAppStatus, because we check on resume, this is not needed
|
This fix is far from polished, but I think I've unblocked React routing from the profile at this point. The migration that I was hoping to do of some of the logical code from |
|
We just talked about some ways to work through this "alert" issue:
|
the debouncing patch for toggleLowAccuracy is no longer needed!
since these arguments are optional, and we render based on their existence, set the default to undefined
no longer need to debounce, so no need to keep code around
to start decluttering `ProfileSettings` move the component to the ControlSyncHelper then import it into `Profile Settings` by exporting a component, we can wrap the popups and their states into one place
this was one way I attempted to control the ControlSettings, but moved to set/get from a central location (plugin) for one-off changes (toggles) and using the local config, and saving once, when editing with the settings editor modal
This is what I've done, for now, with force sync. Now I'm realizing "endForceSync" (ends a trip then forces a sync) relies on that function too. I could easily remedy this if I put the rows next to each other. Currently, |
moving endForceSync and forceTransition into their respective helpers
Can you clarify why do they need to be next to each other given that they are separate components? Having said that, I am OK with rmoving "Force Sync" into the developer zone. The only reason it is outside right now is because one of the early studies, which only ran for a week, wanted to make sure that participants had uploaded all their data, so wanted to "force sync" at the end of the study. We haven't had thta requirement for a while now, probably because we are so focused on longitudinal data collection. So I think it is fine to change it now. |
While they are separate components,
By calling |
for consistency and readability, re-writing the forceSync to rely on await rather than chained .then statements
for readability, switch from chained .then statements to using await calls
shankari
changed the base branch from
label_dashboard_profile_sept_2023
to
onboarding_routing_sept_2023
| const [showingSensed, setShowingSensed] = useState(false); | ||
| const [showingLog, setShowingLog] = useState(false); | ||
| const [editSync, setEditSync] = useState(false); | ||
| const [editCollection, setEditCollection] = useState(false); |
There was a problem hiding this comment.
I'm a bit confused about the interaction between editCollection and editCollectionConfig.
I see that there is an editCollectionConfig which sets editCollection to true, which presumably opens the popup for the edit. But then what does editCollection do, and how does it interact with the refreshCollectionSettings in the useEffect below, and in the control.update.complete event broadcast from the helper
There was a problem hiding this comment.
The control.update.complete event broadcast cannot be relied on anymore, as it was intertwined with Angular routing.
editCollectionConfig is what is called when the user clicks the pencil, setting editCollection to true and showing the editor menu. When the menu is closed, we want refreshCollectionSettings to be called so the useEffect calls refreshCollectionSettings if editCollection changes (it will become false when the editor is closed). This way, the UI for the settings is updates without having to rely on the broadcast events from angular.
Does that help explain things?
We are almost to the point of being able to implement React routing, but first the parts of ProfileSettings that rely on
settingsScope(angular) must be convertedthis includes:
ControlCollectionHelper,ControlSyncHelper,showLog, andshowSensed