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

🏗️ Major Refactor to Remove Label Button Services #1086

Merged
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
3b5e169
Merge branch 'master' of https://github.com/e-mission/e-mission-phone…
JGreenlee Oct 9, 2023
dc28f99
Merge branch 'input-matcher-rewrite' of https://github.com/jiji14/e-m…
JGreenlee Oct 25, 2023
8f76ecf
Merge branch 'input-matcher-rewrite' of https://github.com/jiji14/e-m…
JGreenlee Oct 25, 2023
8712380
use section summary for getDetectedModes
JGreenlee Oct 25, 2023
f7716cf
refactor filters: prep to remove buttons services
JGreenlee Oct 29, 2023
4d48aee
map user inputs instead of populating tlEntry objs
JGreenlee Oct 30, 2023
3855a03
remove btn services and survey.ts
JGreenlee Oct 30, 2023
40d3f75
add back verifiability with verifiabilityForTrip()
JGreenlee Oct 30, 2023
49ff385
remove populateCompositeTrips; update types
JGreenlee Oct 30, 2023
a34c219
TripCard: don't show footer if no notes
JGreenlee Oct 30, 2023
c262f6a
remove unused vars in LabelTab
JGreenlee Oct 30, 2023
9009517
fix label button showing before appConfig loaded
JGreenlee Oct 31, 2023
fb46586
combine processed+unprocessed survey responses
JGreenlee Oct 31, 2023
af5948b
move LabelTabContext to own file to expand types
JGreenlee Oct 31, 2023
d96ff8d
expand types
JGreenlee Oct 31, 2023
22b80f9
fix diaryHelper.test.ts
JGreenlee Oct 31, 2023
081b142
Merge branch 'input-matcher-rewrite' of https://github.com/jiji14/e-m…
JGreenlee Nov 1, 2023
956f2d9
Merge remote-tracking branch 'upstream/service_rewrite_2023' into rew…
JGreenlee Nov 2, 2023
5110845
apply prettier to PR #1086
JGreenlee Nov 2, 2023
1f952f7
remove 'locales' submodule
JGreenlee Nov 3, 2023
1b01692
fix syntax errors and apply prettier
JGreenlee Nov 3, 2023
5eb8d8f
Merge branch 'service_rewrite_2023' of https://github.com/e-mission/e…
JGreenlee Nov 3, 2023
24b34ee
clean up confirmHelper
JGreenlee Nov 3, 2023
55c313d
add tests for confirmHelper
JGreenlee Nov 3, 2023
a60b1e2
Merge branch 'service_rewrite_2023' of https://github.com/e-mission/e…
JGreenlee Nov 10, 2023
d3e6af2
fix en.json -> "questions"
JGreenlee Nov 10, 2023
69b18cb
remove duplicate 'verifiability' check
JGreenlee Nov 10, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix syntax errors and apply prettier
Prettier was not running on these files because they had syntax errors from bad merge conflicts. Fixes syntax on these and runs Prettier to clean up
  • Loading branch information
JGreenlee committed Nov 3, 2023
commit 1b0169208dccdf913ad2f2bee0aa72a89aa09f8d
41 changes: 23 additions & 18 deletions www/js/diary/cards/TripCard.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,22 +7,22 @@
import React, { useContext } from 'react';
import { View, useWindowDimensions, StyleSheet } from 'react-native';
import { Text, IconButton } from 'react-native-paper';
import LeafletView from "../../components/LeafletView";
import { useTranslation } from "react-i18next";
import MultilabelButtonGroup from "../../survey/multilabel/MultiLabelButtonGroup";
import UserInputButton from "../../survey/enketo/UserInputButton";
import useAppConfig from "../../useAppConfig";
import AddNoteButton from "../../survey/enketo/AddNoteButton";
import AddedNotesList from "../../survey/enketo/AddedNotesList";
import { getTheme } from "../../appTheme";
import { DiaryCard, cardStyles } from "./DiaryCard";
import { useNavigation } from "@react-navigation/native";
import { useAddressNames } from "../addressNamesHelper";
import LeafletView from '../../components/LeafletView';
import { useTranslation } from 'react-i18next';
import MultilabelButtonGroup from '../../survey/multilabel/MultiLabelButtonGroup';
import UserInputButton from '../../survey/enketo/UserInputButton';
import useAppConfig from '../../useAppConfig';
import AddNoteButton from '../../survey/enketo/AddNoteButton';
import AddedNotesList from '../../survey/enketo/AddedNotesList';
import { getTheme } from '../../appTheme';
import { DiaryCard, cardStyles } from './DiaryCard';
import { useNavigation } from '@react-navigation/native';
import { useAddressNames } from '../addressNamesHelper';
import LabelTabContext from '../LabelTabContext';
import useDerivedProperties from "../useDerivedProperties";
import StartEndLocations from "../components/StartEndLocations";
import ModesIndicator from "./ModesIndicator";
import { useGeojsonForTrip } from "../timelineHelper";
import useDerivedProperties from '../useDerivedProperties';
import StartEndLocations from '../components/StartEndLocations';
import ModesIndicator from './ModesIndicator';
import { useGeojsonForTrip } from '../timelineHelper';

type Props = { trip: { [key: string]: any } };
const TripCard = ({ trip }: Props) => {
Expand All @@ -41,7 +41,11 @@ const TripCard = ({ trip }: Props) => {
let [tripStartDisplayName, tripEndDisplayName] = useAddressNames(trip);
const navigation = useNavigation<any>();
const { labelOptions, timelineLabelMap, timelineNotesMap } = useContext(LabelTabContext);
const tripGeojson = useGeojsonForTrip(trip, labelOptions, timelineLabelMap[trip._id.$oid]?.MODE?.value);
const tripGeojson = useGeojsonForTrip(
trip,
labelOptions,
timelineLabelMap[trip._id.$oid]?.MODE?.value,
);

const isDraft = trip.key.includes('UNPROCESSED');
const flavoredTheme = getTheme(isDraft ? 'draft' : undefined);
Expand Down Expand Up @@ -98,7 +102,8 @@ const TripCard = ({ trip }: Props) => {
displayEndName={tripEndDisplayName}
/>
</View>
<View style={[cardStyles.panelSection, {marginBottom: 0}]}>{/* mode and purpose buttons / survey button */}
<View style={[cardStyles.panelSection, { marginBottom: 0 }]}>
{/* mode and purpose buttons / survey button */}
{appConfig?.survey_info?.['trip-labels'] == 'MULTILABEL' && (
<MultilabelButtonGroup trip={trip} />
)}
Expand Down Expand Up @@ -128,7 +133,7 @@ const TripCard = ({ trip }: Props) => {
)}
</View>
</View>
{timelineNotesMap[trip._id.$oid]?.length &&
{timelineNotesMap[trip._id.$oid]?.length && (
<View style={cardStyles.cardFooter}>
<AddedNotesList timelineEntry={trip} additionEntries={timelineNotesMap[trip._id.$oid]} />
</View>
Expand Down
76 changes: 49 additions & 27 deletions www/js/diary/details/LabelDetailsScreen.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,26 +2,33 @@
listed sections of the trip, and a graph of speed during the trip.
Navigated to from the main LabelListScreen by clicking a trip card. */

import React, { useContext, useState } from "react";
import { View, Modal, ScrollView, useWindowDimensions } from "react-native";
import { PaperProvider, Appbar, SegmentedButtons, Button, Surface, Text, useTheme } from "react-native-paper";
import React, { useContext, useState } from 'react';
import { View, Modal, ScrollView, useWindowDimensions } from 'react-native';
import {
PaperProvider,
Appbar,
SegmentedButtons,
Button,
Surface,
Text,
useTheme,
} from 'react-native-paper';
import LabelTabContext from '../LabelTabContext';
import LeafletView from "../../components/LeafletView";
import { useTranslation } from "react-i18next";
import MultilabelButtonGroup from "../../survey/multilabel/MultiLabelButtonGroup";
import UserInputButton from "../../survey/enketo/UserInputButton";
import { useAddressNames } from "../addressNamesHelper";
import { SafeAreaView } from "react-native-safe-area-context";
import useDerivedProperties from "../useDerivedProperties";
import StartEndLocations from "../components/StartEndLocations";
import { useGeojsonForTrip } from "../timelineHelper";
import TripSectionsDescriptives from "./TripSectionsDescriptives";
import OverallTripDescriptives from "./OverallTripDescriptives";
import ToggleSwitch from "../../components/ToggleSwitch";
import useAppConfig from "../../useAppConfig";
import LeafletView from '../../components/LeafletView';
import { useTranslation } from 'react-i18next';
import MultilabelButtonGroup from '../../survey/multilabel/MultiLabelButtonGroup';
import UserInputButton from '../../survey/enketo/UserInputButton';
import { useAddressNames } from '../addressNamesHelper';
import { SafeAreaView } from 'react-native-safe-area-context';
import useDerivedProperties from '../useDerivedProperties';
import StartEndLocations from '../components/StartEndLocations';
import { useGeojsonForTrip } from '../timelineHelper';
import TripSectionsDescriptives from './TripSectionsDescriptives';
import OverallTripDescriptives from './OverallTripDescriptives';
import ToggleSwitch from '../../components/ToggleSwitch';
import useAppConfig from '../../useAppConfig';

const LabelScreenDetails = ({ route, navigation }) => {

const { timelineMap, labelOptions, timelineLabelMap } = useContext(LabelTabContext);
const { t } = useTranslation();
const { height: windowHeight } = useWindowDimensions();
Expand All @@ -32,9 +39,13 @@ const LabelScreenDetails = ({ route, navigation }) => {
const { displayDate, displayStartTime, displayEndTime } = useDerivedProperties(trip);
const [tripStartDisplayName, tripEndDisplayName] = useAddressNames(trip);

const [ modesShown, setModesShown ] = useState<'labeled'|'detected'>('labeled');
const tripGeojson = useGeojsonForTrip(trip, labelOptions, modesShown=='labeled' && timelineLabelMap[trip._id.$oid]?.MODE?.value);
const mapOpts = {minZoom: 3, maxZoom: 17};
const [modesShown, setModesShown] = useState<'labeled' | 'detected'>('labeled');
const tripGeojson = useGeojsonForTrip(
trip,
labelOptions,
modesShown == 'labeled' && timelineLabelMap[trip._id.$oid]?.MODE?.value,
);
const mapOpts = { minZoom: 3, maxZoom: 17 };

const modal = (
<Modal visible={true}>
Expand Down Expand Up @@ -82,13 +93,24 @@ const LabelScreenDetails = ({ route, navigation }) => {

{/* If trip is labeled, show a toggle to switch between "Labeled Mode" and "Detected Modes"
otherwise, just show "Detected" */}
{timelineLabelMap[trip._id.$oid]?.MODE?.value ?
<ToggleSwitch onValueChange={v => setModesShown(v)} value={modesShown} density='medium'
buttons={[{label: t('diary.labeled-mode'), value: 'labeled'}, {label: t('diary.detected-modes'), value: 'detected'}]} />
:
<Button mode='outlined' compact={true} textColor={colors.onBackground}
style={{height: 32}} contentStyle={{height:30}}>
{ t('diary.detected-modes') }
{timelineLabelMap[trip._id.$oid]?.MODE?.value ? (
<ToggleSwitch
onValueChange={(v) => setModesShown(v)}
value={modesShown}
density="medium"
buttons={[
{ label: t('diary.labeled-mode'), value: 'labeled' },
{ label: t('diary.detected-modes'), value: 'detected' },
]}
/>
) : (
<Button
mode="outlined"
compact={true}
textColor={colors.onBackground}
style={{ height: 32 }}
contentStyle={{ height: 30 }}>
{t('diary.detected-modes')}
</Button>
)}

Expand Down
53 changes: 29 additions & 24 deletions www/js/diary/services.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,30 +4,35 @@ import angular from 'angular';
import { getConfig } from '../config/dynamicConfig';
import { getRawEntries } from '../commHelper';

angular.module('emission.main.diary.services', ['emission.plugin.logger',
'emission.services'])
.factory('Timeline', function($http, $ionicLoading, $ionicPlatform, $window,
$rootScope, UnifiedDataLoader, Logger, $injector) {
var timeline = {};
// corresponds to the old $scope.data. Contains all state for the current
// day, including the indication of the current day
timeline.data = {};
timeline.data.unifiedConfirmsResults = null;
timeline.UPDATE_DONE = "TIMELINE_UPDATE_DONE";

// DB entries retrieved from the server have '_id', 'metadata', and 'data' fields.
// This function returns a shallow copy of the obj, which flattens the
// 'data' field into the top level, while also including '_id' and 'metadata.key'
const unpack = (obj) => ({
...obj.data,
_id: obj._id,
key: obj.metadata.key,
origin_key: obj.metadata.origin_key || obj.metadata.key,
});

timeline.readAllCompositeTrips = function(startTs, endTs) {
$ionicLoading.show({
template: i18next.t('service.reading-server')
angular
.module('emission.main.diary.services', ['emission.plugin.logger', 'emission.services'])
.factory(
'Timeline',
function (
$http,
$ionicLoading,
$ionicPlatform,
$window,
$rootScope,
UnifiedDataLoader,
Logger,
$injector,
) {
var timeline = {};
// corresponds to the old $scope.data. Contains all state for the current
// day, including the indication of the current day
timeline.data = {};
timeline.data.unifiedConfirmsResults = null;
timeline.UPDATE_DONE = 'TIMELINE_UPDATE_DONE';

// DB entries retrieved from the server have '_id', 'metadata', and 'data' fields.
// This function returns a shallow copy of the obj, which flattens the
// 'data' field into the top level, while also including '_id' and 'metadata.key'
const unpack = (obj) => ({
...obj.data,
_id: obj._id,
key: obj.metadata.key,
origin_key: obj.metadata.origin_key || obj.metadata.key,
});

timeline.readAllCompositeTrips = function (startTs, endTs) {
Expand Down
112 changes: 70 additions & 42 deletions www/js/survey/multilabel/MultiLabelButtonGroup.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,16 +2,32 @@
In the default configuration, these are the "Mode" and "Purpose" buttons.
Next to the buttons is a small checkmark icon, which marks inferrel labels as confirmed */

import React, { useContext, useEffect, useState, useMemo } from "react";
import { getAngularService } from "../../angular-react-helper";
import { View, Modal, ScrollView, Pressable, useWindowDimensions } from "react-native";
import { IconButton, Text, Dialog, useTheme, RadioButton, Button, TextInput } from "react-native-paper";
import DiaryButton from "../../components/DiaryButton";
import { useTranslation } from "react-i18next";
import LabelTabContext from "../../diary/LabelTabContext";
import { displayErrorMsg, logDebug } from "../../plugin/logger";
import { getLabelInputDetails, getLabelInputs, inferFinalLabels, labelInputDetailsForTrip, labelKeyToRichMode, readableLabelToKey, verifiabilityForTrip } from "./confirmHelper";
import useAppConfig from "../../useAppConfig";
import React, { useContext, useEffect, useState, useMemo } from 'react';
import { getAngularService } from '../../angular-react-helper';
import { View, Modal, ScrollView, Pressable, useWindowDimensions } from 'react-native';
import {
IconButton,
Text,
Dialog,
useTheme,
RadioButton,
Button,
TextInput,
} from 'react-native-paper';
import DiaryButton from '../../components/DiaryButton';
import { useTranslation } from 'react-i18next';
import LabelTabContext from '../../diary/LabelTabContext';
import { displayErrorMsg, logDebug } from '../../plugin/logger';
import {
getLabelInputDetails,
getLabelInputs,
inferFinalLabels,
labelInputDetailsForTrip,
labelKeyToRichMode,
readableLabelToKey,
verifiabilityForTrip,
} from './confirmHelper';
import useAppConfig from '../../useAppConfig';

const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => {
const { colors } = useTheme();
Expand Down Expand Up @@ -78,40 +94,52 @@ const MultilabelButtonGroup = ({ trip, buttonsInline = false }) => {
}

const tripInputDetails = labelInputDetailsForTrip(timelineLabelMap[trip._id.$oid], appConfig);
return (<>
<View style={{flexDirection: 'row', alignItems: 'center'}}>
<View style={{flex: 1, flexDirection: buttonsInline ? 'row' : 'column', columnGap: 8}}>
{Object.keys(tripInputDetails).map((key, i) => {
const input = tripInputDetails[key];
const inputIsConfirmed = timelineLabelMap[trip._id.$oid]?.[input.name];
const inputIsInferred = inferFinalLabels(trip, timelineLabelMap[trip._id.$oid])[input.name];
let fillColor, textColor, borderColor;
if (inputIsConfirmed) {
fillColor = colors.primary;
} else if (inputIsInferred) {
fillColor = colors.secondaryContainer;
borderColor = colors.secondary;
textColor = colors.onSecondaryContainer;
}
const btnText = inputIsConfirmed?.text || inputIsInferred?.text || input.choosetext;
return (
<>
<View style={{ flexDirection: 'row', alignItems: 'center' }}>
<View style={{ flex: 1, flexDirection: buttonsInline ? 'row' : 'column', columnGap: 8 }}>
{Object.keys(tripInputDetails).map((key, i) => {
const input = tripInputDetails[key];
const inputIsConfirmed = timelineLabelMap[trip._id.$oid]?.[input.name];
const inputIsInferred = inferFinalLabels(trip, timelineLabelMap[trip._id.$oid])[
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ditto

input.name
];
let fillColor, textColor, borderColor;
if (inputIsConfirmed) {
fillColor = colors.primary;
} else if (inputIsInferred) {
fillColor = colors.secondaryContainer;
borderColor = colors.secondary;
textColor = colors.onSecondaryContainer;
}
const btnText = inputIsConfirmed?.text || inputIsInferred?.text || input.choosetext;

return (
<View key={i} style={{flex: 1}}>
<Text>{t(input.labeltext)}</Text>
<DiaryButton fillColor={fillColor} borderColor={borderColor}
textColor={textColor} onPress={(e) => setModalVisibleFor(input.name)}>
{ t(btnText) }
</DiaryButton>
</View>
)
})}
</View>
{verifiabilityForTrip(trip, timelineLabelMap[trip._id.$oid]) == 'can-verify' && (
<View style={{marginTop: 16}}>
<IconButton icon='check-bold' mode='outlined' size={18} onPress={verifyTrip}
containerColor={colors.secondaryContainer}
style={{width: 24, height: 24, margin: 3, borderColor: colors.secondary}} />
return (
<View key={i} style={{ flex: 1 }}>
<Text>{t(input.labeltext)}</Text>
<DiaryButton
fillColor={fillColor}
borderColor={borderColor}
textColor={textColor}
onPress={(e) => setModalVisibleFor(input.name)}>
{t(btnText)}
</DiaryButton>
</View>
);
})}
</View>
{verifiabilityForTrip(trip, timelineLabelMap[trip._id.$oid]) == 'can-verify' && (
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did we ever figure out the performance difference between calling functions and using fields in the TSX? As I said earlier, I am concerned that the overhead of a function call for each trip every time we scroll, for example. Does React handle this properly with caching?

Copy link
Collaborator Author

@JGreenlee JGreenlee Nov 10, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Theoretically, in a React project I think the difference should be negligible because React components are designed to only re-render when their state, or any stateful props, are explicitly updated. That's why React doesn't like mutations - if a value changes by any other way, the React component will not reflect the change because it doesn't know to re-render itself.
In contrast, I think the old Angular app was just constantly re-rendering things.

One caveat here though: the way FlashList works is complicated behind the scenes. It's a recycled list view where things DO have to get recomputed if they go off the screen and go back onto the screen. But there are things you can do to optimize this, which I've done my best of so far.
Some info: https://shopify.github.io/flash-list/docs/recycling https://shopify.github.io/flash-list/docs/fundamentals/performant-components/

<View style={{ marginTop: 16 }}>
<IconButton
icon="check-bold"
mode="outlined"
size={18}
onPress={verifyTrip}
containerColor={colors.secondaryContainer}
style={{ width: 24, height: 24, margin: 3, borderColor: colors.secondary }}
/>
</View>
)}
{trip.verifiability === 'can-verify' && (
<View style={{ marginTop: 16 }}>
<IconButton
Expand Down