📈 👔 ✅ Dashboard rewrite (complete) + profile logic improvements + initial testing framework! #1046
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
- replaced normal details view of 'My distance' with chart
- next steps
- create button to switch between chart and details view in 'my distance' card
- Figure out why all data is showing up unlabled
- Allow the axis to switch easily
- right now, setting isHorizontal doesn't completely switch data, only thedirection of the bars
- would be nice to flip data axis and bar axis simultaneously
- one (MetricDetails) is going to be a copy of the original metric details view
- the next (MetricsCard) is going to be a container to have both the BarChart component, and the MetricDetails component
- Will be a react component, rather than function, and will hold onto the state of whether or not the barchart or metricdetails will be showing
- next steps
- add a button to switch between the two views
- allows the card to keep track of whether or not it has a BarChart or MetricDetails child component
- next step
- Add button
- next step
- format and style button
- button allows switching between the BarChart and the MetricDetails card - button icon switches depending on the card represented
- removed children property from card - removed leftover code from copying from BarChart.tsx to MetricsCard and MetricDetails - Removed unecessary import statements
-- also switched main-metrics html so that only the MetricsCard element is held in the ion-slide element, without any extra divs or headers
…sion/e-mission-phone into dashboard-rewrite
The new dashboard will have the MetricsTab component as its entry point - main-metrics.html will no longer be rendered
Replacing main-metrics, this component represents the entire Dashboard tab and will get metrics data from the server, store it as state, and pass it down to child components like MetricsCard. There is also a date picker at the top (TODO) and a refresh button. The logic for fetching metrics has been modernized into modern JS (async/await) and uses Luxon instead of Moment. I tested to make sure the timestamps of the resulting queries are the same using Luxon as they were with Moment.
This typing is based on the data we receive from the server. For each metric, we receive an array containing "days" of metric data. Each day has a few props (ts, fmt_time, nUsers, local_dt) plus any number of properties with the `label_` prefix that represent the measurment for a particular label. Having this be type-safe from the start should make it much easier to realized the desired visualizations!
MetricsCard will now accept 'metricDataDays' as a prop and derive the chart data from this. We can employ `useMemo` for this so that iff metricDataDays changes, chartData updates. Prior to this, MetricsCard had an IconButton on the right side allowing users toggle between 'details' and 'graph' view. To be clearer to the user, I have changed this to a SegmentedButtons. This way, users see both their options side by side and can select the desired one. A few colors were added in appTheme.ts - surfaceDisabled and onSurfaceDisabled. These are colors that already exist in the React Native Paper default theme, but we are overriding for our own purposes. In the SegmentedButtons on MetricsCard, the surfaceDisabled color is used for the button that is not checked
With the new MetricsTab, MetricsDetails will receive 'metricDataDays', sum up the totals for each label, and display these in a 2-column. The display is the same as the old dashboard, but now the logic to sum up the values is here in this component instead of jumbled in with everything else.
MetricsDateSelect will allow users to select a date range. It appears as a NavBarButton, like the label screen date picker, and also uses a DatePickerModal. However, this one works differently because users can select a range of days instead of just one. The selected range is stored in LabelTab as `dateRange` - once this range is loaded, all the displayed metrics will update to reflect the new data. `dateRange` is an array of two Luxon DateTime objects, which works well to bridge between the JS Date objects (used by RN Paper Dates) and Unix timestamps (used by e-mission-server / for any network calls)
The chart does not need so much padding - we want to be able to show as much detail as we can. MetricDetails should show with 2 columns, but could use some padding in between. So let's remove all the padding from the card content since its children will have padding anyway.
The "active minutes" should display the cumulative duration for modes that are considered "active" - currenty this is just 'walk' and 'bike'; I am not sure if anything else will count. This should include a graph later on, but right now it just lists out the minutes for each mode.
The same type will be used for both user metrics and aggregate metrics, so its naming should be more generic.
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.
It's not really necessary for MetricDetails to be its own component - it is actually simpler to just include these few lines of code in MetricsCard. Then MetricsCard will handle the memoized computation either way, whether it's `chartData` or `metricSumValues` that is needed (depending on whether 'graph' or 'details' is active)
It was incorporated into MetricsCard and is no longer a standalone component
…sion/e-mission-phone into dashboard-rewrite
It is useful to have the format function (which handles rounding to 2 or 3 sig figs) generic enough to work for any unit. We can also have it use Intl.NumberFormat to work better with other languages (other languages might use commas and periods differently, or have a different decimal marking altogether)
Using a prop passed to MetricsCard called unitFormatFn, we will format the raw numerical values as human-readable strings in the correct unit of measurement.
Added a second pair of SegmentedButtons to allow toggling between 'user' and 'aggregate' metrics. So now there are two toggles - instead of repeating all that SegmentedButtons code, I extracted it out into a new ToggleSwitch component, which is a generic wrapper around SegmentedButtons for our particular uses. Each MetricsCard now accepts both userMetricsDays and aggMetricsDays, instead of just one metricDataDays, and it determines which to use based on the value of the toggleswitch - "populationMode"
viewMode was not in the dependency array, so if it was changed in isolation, metricSumValues would not recompute.
If the scale of the graph is too large, the 2030 and 2050 guidelines labels might overlap. In that case, we should show the 2030 label on top. Also, the colors are kind of hard to see with the bars having their new gradient color scheme. So for the colors of the lines, let's just darken the yellow, and bump up the saturation of both.
found a bug where the text for the group was being calculated off of user metrics!
to avoid showing metrics for partial weeks, check the length before displaying.
moving endForceSync and forceTransition into their respective helpers
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
we've moved away from showing "calories burned in active travel" and started to show "minutes spent in active travel" Therefore, we no longer need the keys about calories, calibrating to user date (already left profile) or comparing the calories to food.
…023' into dashboard-rewrite
Profile changes to unblock routing
…ssion-phone into dashboard-rewrite
also tested this code by running in emulator to ensure no errors
since we have room in the code, clarify what we mean by referencing "how many days ago" the calculations cover
if the change is infinite, show that. If the change is infinite, and infinitely uncertain (-inf/+inf) don't show, and otherwise show the finite range
📊 Rewrite of the New Dashboard Tab
Described in e-mission/e-mission-docs#986 (comment), we have a production-specific error where the needed ChartJS elements are not recognized as being registered. Likely an incompatibility between ChartJS and Webpack. As a workaround, we will register all of the `registerables`. Unfortunately, we will no longer benefit from tree-shaking with this approach.
Register all ChartJS elements as workaround
In AppStatusModal, we listen for the value of overallStatus and if it is false, we trigger the permissions popup. But there is an issue where the AppStatusModal sometimes shows up even if all the checks are green. This is because it can take some time for the checks to initialize and return back their value. During this time, we want to have a sort of 'undetermined' state for overallStatus, so that we don't show the popup prematurely. We'll do this by allowing overallStatus to be either true/false OR undefined. If there are no checks yet, overallStatus will be undefined and we won't show the popup yet. Once the checks are initialized, we'll be able to determine the status and it will return true or false. For the individual checks themselves, they also need to be able to report an 'undetermined' state. If their statusStates are false by default, and are false before they have actually been determined, then overallStatus will also be false while the check is still ongoing. So we should remove the initial 'false' condition from the few checks that had it, allowing the initial to be 'undefined', and when we iterate through the checks, we will explicitly check if `statusState === false` instead of just checking if `!statusState`. Result of this is that AppStatusModal only shows if any permissions are broken. Tested by loading the app, with all of the correct permissions, 20 times both before and after the change. Before the change, 2 of the attempts resulted in the AppStatusModal incorrectly appearing. After the change, the AppStatusModal did not appear on any of the 20 attempts. After this, I disallowed the Location permission and the modal did still appear as expected.
on some occasions, the checkList was failing to be initialized. Upon investigation, this turned out to be because platform and osver were "" and 0, respectively. The underlying issue is that they were held in useState, which is updated asyncronously. useRef is more appropriate here, as the platform and osver only need to be set once, and are not anticipated to change. The access changed, as useRef variables are read and set from "var.current" Before these changes, roughly 1/3 of the time, the platform and osver were not set yet, and so the checks were not set up as the platform was interpreted as not valid. After the change, the platform and osver are always set properly in time to call createChecklist. I also updated the useEffect that calls the createChecklist to do so if checkList does not exist or has a length of 0, rather than the else that I had before.
Something I noticed while adding new tests - modeColors is used in diaryHelper.test.ts, but it isn't exported from diaryHelper.ts. I think this was due to a bad merge conflict. Simply exporting it fixes the tests, so I'm just going to bundle in that fix here.
I forgot to remove cch from one place in this file when I translated it from it's former angular version, which was causing the problem with android devices and the medium accuracy toggle While here, resolve the syntax highlighting related to window.cordova by replacing it with window['cordova'], not fixing this syntax in the first place likely contributed to me overlooking this error.
Fixes for End of Sept RC
the-bay-kay
added a commit
to the-bay-kay/e-mission-phone
that referenced
this pull request
Feb 9, 2024
See Issue [e-mission#1046](e-mission/e-mission-docs#1046 (comment)) for more details. This commit adds the Dev-Zone settings option, which will eventually link to the Bluetooth Scanner page. TODO: - Add i18next translation for control.bluetooth
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.
Particularly excited about finally getting some unit tests in place...