-
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
Merge 1.4.8 to master #1035
Merged
Merged
Merge 1.4.8 to master #1035
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
migrating settings.auth out of general-settings and into ProfileSettings
logout now also uses a Modal in Profile Settings
moved endForceSync over to profile settings
removal of some old code in general-settings, take care of some formatting in ProfileSettings
also app version was migrated here, still some places that it listens to call in general-settings, so need to migrate those soon
now a modal pops up instead of an AlertBar to take user input and act accordingly -- hard to replicate and test behavior in the emulator so more testing needed
Discovered Platform.OS will always return web right now, so had to switch for accessing device type form cordova
leveraging the appConfig listener to update the settings once they are ready
mostly migrated consent checking now - put off the reconsent proceedure until I figure out state.go
now using a reactNative paper dates to handle downloading the data dump -- noting that we could switch to downloading a range when we update the ControlHelper
all settings are stored in state in ProfileSettings! So no longer need to import them from the scope of general-settings
this element shows the status of the permissions and now gets called when you check the app status in settings - eventually need to be able to launch conditionally like it is in general-settings - also need to add in the permissions bars
deleting old code in general-settings fixing spacings adding comments about migration fix issue with not using authSettings
i18n.language is now used to access the locale that get's used for the date picker
only allow enable button when overallStatus is good
… profile_logic_migration
Logout was not logging the user out, it was just refreshing. Now the logout works
creating a React version of the permissions checks, once this is ironed out, it should be helpful in the initial login as well
This data is soon-to-be unneeded because we are eliminating calorie calculations from the Dashboard during upcoming migration - so rather than fix, we are taking it out
using a View as a wrapper to elements in ProfileSettings in attempt to get alertbars to work
icons not having defaults was breaking everything, but that's not actually the real problem
On the label screen, if there are errors we want to show user-facing popups so people can report the errors effectively and we can address them. We will do this whenever trips are 1) loaded and 2) populated, since these areas were missing any kind of error catches logger.ts was slightly modified to show a 'header' in the window.alert popup, with the title of the error between bars like: ━━━━ <Error title> ━━━━ <Error body...> This is fairly primitive but I think it works for now; we can later explore "Alert" libraries from the community if we want to.
e-mission/e-mission-docs#966 We often mistakenly get 'stubs' of trips while people are staying at one place (just sitting at home, etc) - the sensing detects a transition but it is not a real trip. We filter for these on the server and usually they don't make it through the pipeline, but we also want to filter them out for draft trips (while they exist on the phone/ before the server filters them) There is already a filter step here to filter out any 'undefined' entries. Let's tack on some extra criteria so we also filter anything with distance <100m and duration <5 min. Note: this file is going to be rewritten before too long so I'm not going to clean anything else up while I'm here.
If a survey response is recorded from UserInputButton on the Label details screen, the button should reflect the newly recorded response. This wasn't working because UserInputButton did not call repopulateTimelineEntry, allowing the entire Label tab context to update (which is the paradigm we are using for MultilabelButtonGroup and AddNoteButton. Instead we were just writing into the timelineEntry object on the 'userInput' field (the old/Angular way of doing things - and React is not aware of this mutation happening) Unifying this with the approach of AddNoteButton resolves the issue. Also refactored UserInputButton in general: we do not need both displayLabel and isFilled as state. If no response label exists, that's the same as isFilled=false The responseLabel is derived from the properties within timelineEntry - so useMemo is more efficient than useState+useEffect here. Lastly, let's ensure the log statements in onResponseSaved are using logger.ts, not plain console statements.
Unprocessed ('draft') trips don't have any sections but we still want to show descriptives for them. We'll construct a single section to be displayed, based on the overall trip, in much the same way that we do when we're showing the labeled mode instead of the detected modes. We don't know what mode draft trips are, so we won't have a base mode to use for icon+color - we can fallback to UNPROCESSED (which shows a question mark in grey). Draft trips won't have 'text' either - it will stay undefined and not show up in the UI. Once a draft trip is labeled, it will be treated the same as any other labeled trip.
Draft trips have no detected modes, and so ModesIndicator should not render anything - not even "Detected: ". There was a mistake in the conditional logic here on line 34. The comment on line 35 clearly describes the desired condition, which, once implemented, does achieve the desired result.
When there is only one button on the label details screen, it takes up the full page width, way bigger than it needs to be, and does not necessarily look like a button. To ensure it looks like a button, let's make sure it doesn't get too wide. -Add maxWidth of 200px to DiaryButton. Also margin: auto to keep it centered -Instead of declaring the button style directly inside DiaryButton, we can do this as a function in the StyleSheet, consistent with where we are declaring the other styles
if the trip had no sections and showLabeledMode was true but it was unlabeled, we could end up with undefined baseMode. We should only getBaseModeOfLabeledTrip if showLabeledMode is true AND the trip is labeled.
Draft trips should use the grey-colored 'draft' theme flavor for their details screen as well. We can pass this through as a route param.
Rearranges colors and adds a few new colors to accommodate the trip details page being flavored for draft trips. ToggleSwitch will use elevation level1 instead of elevation level2 - this is more readable on draft details and barely changes anything in the default theme flavor.
prevent the modal from profile and the modal from label from opening at the same time suggestion from: #1009 (comment)
As Jack pointed out in review, I did not need this argument to useTranslation() that allowed me to condense the two parts into one as well
As Jack pointed out in review, we need more permissions on Android to copy text, so, if the platform is android we'll do nothing here to avoid breaking over insufficient permissions
Migration of Profile Logic
🐛 Label Screen Fixes, Sept 2023
This setTime action is broken, so we must remove it from the notifications We can restore this is a few weeks after we migrate to React routing
to remedy a lag in the displayed notification time, set the notification settings to refresh when the uiConfig changes, this allows the settings to refresh after it is properly set, and prevents the initially blank reminder time I was seeing
lingering code pending update to Samsung situation -- removed since it isn't used at all, and will add what is needed later
After reports of the instructions being confusing on Android 13 - update text to specify more closely what needs to be changed on 13 in particular This app (https://www.followmee.com/FAQ.aspx?t=androidunusedapp#) had an instructions page that explained the process for different Android OS well
move appVersion to be more visible to users also caught and fixed an error where appVersion was held as an object and not a string
we should not allow users to download data from prior to the beginning of their study The month was setting as 1 in advance of the config, so I adjusted manually with the "-1"
the styling on the opcode modal was not consistent with the other modals, I remedied this by passing the style in from ProfileSettings as a parameter
🧹 Profile cleanup
@JGreenlee @Abby-Wheelis @niccolopaganini @sebastianbarry the tests failed after the merge, because the tests call functions that were refactored in the |
We wrote these tests for the functions in diaryHelper based on master, but an incoming branch `label_dashboard_profile_sept_2023` refactored diaryHelper to support base modes and display detected modes, and so the functions are slightly different and the tests need to be updated. Fortunately, the new functions serve basically the same purpose, and for testing we can pretty much just swap out by changing the names. The colors did need to be updated though-- so let's just export/import modeColors so we can directly compare against the colors defined in diaryHelper.
Fixed; Tests pass with #1036 |
fix diaryHelper tests: update to refactored fns
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.
High level changes: