Skip to content
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

trying to fix my branch to be up to date #1

Merged
merged 107 commits into from
Jun 28, 2023

Conversation

Abby-Wheelis
Copy link
Owner

working on incorporating changes from upstream branch, having a hard time

shankari and others added 30 commits June 7, 2023 21:44
Original fix:
1b2611a

Before this, we had

```
$scope.ui_config.intro.app_required = $scope.ui_config?.intro.app_required || $scope.ui_config?.intro.program_or_study == 'program';
```

This worked fine for backward compat, but in the case of a program where the
app was not required, we ended up with

`false || true` which is `true`

Taking out the fancy javascript and going to a simpler, more verbose check of
whether the property is undefined before overriding it
…results

So that we don't confuse Denver CASR beta testers
We will restore this later once we have moved to a paid service
Based on feedback from one of the interns

> We got it working! I think the confusing part was that the "fix" and "refresh" buttons didn't really look like buttons and I missed them the first time around...
🐛 💩 🚸 port fix, temporarily comment out error, make "fix" and "refresh" look like buttons
linking the devapp to where it is mentioned in the readme
added link and mention of downloading the latest release from the devapp repo
fixes e-mission/e-mission-docs#904

The `browser` platform was added in 460f6ee, but could not be located unless `npx cordova prepare` is called.
In the label screen, the timestamps for each trip/place/untracked are shown at the top and bottom of the card. Since this is used in many places, it has been refactored into a reusable React component.
It uses the Badge component from react-native-paper, and applies the appropriate colors from the theme.
This will allow the use of the `useTranslation()` hook in our React components. This is how we will accomplish internationalization in the new React components we are writing.
https://react.i18next.com/latest/usetranslation-hook
MultiLabelButtonGroup.jsx will be the React component that handles the multilabel buttons. This is a React rewrite of multi-label-ui.
The popup menu is a work in progress; it uses a `Menu` from React Native Paper, but it does not render correctly because of the existing DOM structure that is from AngularJS. We need a way for this to appear on top of all other content.
I finally got icons working with React Native Paper components by following the guide from here:
https://callstack.github.io/react-native-paper/docs/guides/react-native-web/

The webpack configuration was more complicated than it needed to be. There was unnecessary config left over from when I tried to get the icons working before, and this can be removed.
As a pleasant side effect, I think bundling is now happening much faster.
At this point, the multilabel buttons are now behaving almost exactly the same as they used to - except rendered in React instead of AngularJS.

The popups are going to stay as ionic for now. For that to work, we use the $ionicPopover service from within React. This works fine for now.
Later, I'd prefer to make these into Menus or Dialogs. But I can't do this until we have some of the upper-level parts of the UI rewritten in React.
The functionality from the multilabel directive has been replaced by the MultilabelButtonGroup component, and the multilabel directive is no longer being used.
Inside `multi-label-ui.js`, the MultiLabelService is still being used, so that file has to stick around for a while longer.
This is not being used. Although this would be the ideal, I abandoned this idea because the Menus will not display correctly until we have migrated upper-level UI components.
The 'secondary' colors are being used for 'inferred' labels. For text readability, I'm using more of an orange than a yellow.
I'm also tweaking the primary colors in accordance so that we overall have a slightly more pleasant palette.
This popup is now triggered from MultilabelButtonGroup and this function is never used anywhere.
Adding TypeScript to the project required a bit of webpack reconfiguration.
It also made bundling extremely slow (~60s). So, I sifted through the webpack.config.js and optimized it as much as possible. Now, bundling is taking about ~12s.
This allows VSCode's IntelliSense to not have so many files to watch
VSCode wanted me to explicitly add 'children' before on these elements, but I think it was due to a bug.
Once I got TypeScript fully configured, and the TypeScript extensions were reloaded, this was deemed unnecessary, and it gave me a warning to remove 'children' from the definition.
This allows us to 'import React from "react"', instead of having to 'import * as React from "react"'.
In this project we want to be consistent about using ES import syntax, preferably treating everything as either a default or named export. esModuleInterop allows us to do this even if the libraries differ.
Since we are going to keep using the ionic popovers until the rest of the label screen is rewritten, we might as well make them look nicer.
This has been replaced by the new checkmark icon in MultilabelButtonGroup.
This component replaces enketo-add-note-button.
Most of the logic is kept the same.
The new AddNoteButton.tsx has a plus sign icon next to the text.
This style tweak is necessary for buttons with icons to show correctly; otherwise the button label overlaps the icon.
MultilabelButtonGroup should show if elementTag is 'multilabel', and the trip button should show if elementTag is 'enketo-trip-button'
This directive has been replaced by AddNoteButton.jsx.
This file also contains EnketoNotesButtonService, which is still being used, so I cannot remove the entire file as of yet.
JGreenlee and others added 27 commits June 22, 2023 23:14
LabelTab will contain the logic that does the heavy lifting of loadings trips, populating their inputs, and filtering them. It will also supply LabelTabContext to all its descendants.
TimelineScrollList will just contain the logic needed to display the trips in a FlashList, plus loading spinners & 'load more' buttons.

infinite_scroll_list.js is not needed anymore.
Since LabelTab is the parent component of everything else in /diary now, it makes sense for this to live at the root of /diary.
This further breaks extracts parts of LabelTab into LabelScreen.
This will allow LabelTab to serve as the root and include a Navigator, which can route to either LabelScreen or LabelDetailsScreen (in progress).

Anything from LabelTab that is used by either LabelScreen or LabelDetailsScreen will have to be exposed in the LabelTabContext.
It is a little annoying to share all these values between components, but it's better than shoving all the logic in one place.

Clicking a trip card does navigate to LabelDetailsScreen, so the routing is working at this point. All that's left to do is rewrite the details page.
This is an issue with ionic, where the content view has actually extended partway into the tabs so that the bottom of the viewable area has been covered up.
This is a temporary override to fix it. Soon the tabs themselves will be in React Native and this will be irrelevant.
We could pass the tripId and then look it up like the old implementation. But React Navigation parameters can be anything, so why not just pass the whole trip object? It's passed by reference so there's no performance downside to this.
I tried to make this a little faster by using Promise.all. It turns out that DynamicConfig actually depends on ionicPlatform.ready() being already done, so they can't overlap in execution after all.
Also including Logger to diary.js. It was formerly included in infinite_scroll_list.js but I removed that, so it needs to be included here.
remnants of code that was extracted to LabelScreen or TimelineScrollList
replaced by LabelDetailsScreen
When we swapped out ng-translate for ng-i18next, this text in particular stopped translating correctly.
I guess it's a slight difference in how the two directives work.

With the emoji inside the element alongside the translation key, I think i18next is actually looking for a translation key that includes the emoji, which does not exist.
To remedy this, let's just use the `| i18next` filter, which allows us to interpolate the translation without the emoji, and then just include the emoji after.
In e-mission#974
e-mission#980
and
e-mission#990

@JGreenlee removed bower and added several dependencies to npm
this commit copies those dependencies to the cordovabuild as well

Testing done:
- Ran npm install

Added the following manual fix

```
diff --git a/www/index.js b/www/index.js
index c2b2bb97..c7b6ae49 100644
--- a/www/index.js
+++ b/www/index.js
@@ -53,8 +53,10 @@ import './js/metrics.js';
 import './js/control/general-settings.js';
 import './js/control/emailService.js';
 import './js/control/uploadService.js';
-import './js/control/collect-settings.js';
-import './js/control/sync-settings.js';
+import '../platforms/android/platform_www/js/control/collect-settings.js';
+import '../platforms/ios/platform_www/js/control/collect-settings.js';
+import '../platforms/android/platform_www/js/control/sync-settings.js';
+import '../platforms/ios/platform_www/js/control/sync-settings.js';
 import './js/metrics-factory.js';
 import './js/metrics-mappings.js';
 import './js/plugin/logger.js';
```

- `npx webpack`
- `npx cordova build`

Launches on both iOS and android with some minor issues
e-mission/e-mission-docs#923
with e-mission/e-mission-server#917, the "sensed mode" (ie BICYCLING, WALKING, etc) will be in `sensed_mode_str`, while `sensed_mode` will just be the enum index. So, we should use `sensed_mode_str` when looking up modes.
Also, we will be replacing Ionicons and Font Awesome icons with Material Community Icons wherever possible. Suitable substitutes have been found for all of these modes.
The details screen will now show percentages for the sensed mode of travel.
If the trip has more than one section, it will list the sections displaying their duration and mode of travel.

I also adjusted margins and spacing throughout the details screen. Refactoring this later to clean it up would be a good idea.
Floating at the top of the map, we will show an indicator that displays the inferred (or sensed) modes of travel and how much of the trip they were used for.
Since this is specific to the TripCard, I didn't see a need to make a separate component for this. If we start needing it other places, we can consider it then.
alphabetize dependencies + update npm 'build' script to use 'npx webpack' instead of just 'webpack'
In the Content Security Policy, img-src was set to accept 'self' and 'ionic: data: https://*.tile.openstreetmap.org'. I believe this was necessary for the purpose of using ui-leaflet.

Now, we have LeafletView, which is independent of Ionic and AngularJS. It retrieves tiles directly from https://tile.openstreetmap.org.
Updating the policy as such resolves the issue.
The `detachPreviousScreen` option is true by default and means that when you navigate to a new screen, the old one is detached so it can be removed from memory.
In the case of the label details screen, we are going to go right back to the main label screen from the details page. So we shouldn't detach it.

This also fixes the issue of the scroll position resetting when going back from the details page.
There were changes upstream while I was working on the PR this is a part of.
When I pulled them back in and resolved merge conflicts, I overlooked some important changes that had happened with cordova plugins and versioning

https://github.com/e-mission/e-mission-phone/pull/993/files#r1244031155
https://github.com/e-mission/e-mission-phone/pull/993/files#r1244628495
https://github.com/e-mission/e-mission-phone/pull/993/files#r1245346066
…gration

🏷️ Migration of Label Screen Components
@Abby-Wheelis Abby-Wheelis merged commit cf2871e into More-Profile-Migrations Jun 28, 2023
Abby-Wheelis pushed a commit that referenced this pull request Dec 28, 2023
Update Type Definitions in diaryTypes.ts
Abby-Wheelis pushed a commit that referenced this pull request Feb 5, 2024
🗓️ Fixed WSOD on Metric Date Selector
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants