e-mission / e-mission-docs

Repository for docs and issues. If you need help, please file an issue here. Public conversations are better for open source projects than private email.
https://e-mission.readthedocs.io/en/latest
BSD 3-Clause "New" or "Revised" License
15 stars 34 forks source link

Need to only show the replaced mode for the intervention that is being evaluated #764

Closed shankari closed 2 years ago

shankari commented 2 years ago

This is a follow-on to https://github.com/e-mission/e-mission-docs/issues/755

In addition to only showing the replaced mode for programs, we should only show it for the intervention that is being evaluated.

shankari commented 2 years ago

Currently going to hardcode the intervention to e-bike. Changes to the dynamic config format depend on #765

shankari commented 2 years ago

ensuring that we display the replaced mode only if the original mode is e-bike is conceptually simple. We keep the replaced mode button under an ng-if which checks the mode button. But there are some complications, notably that we need to adjust the widths if we want to have three buttons there instead of two.

Concretely, if we want to have two buttons, the widths are col-50 and 115px If we want to have three buttons, the widths are col-33 and 80px

shankari commented 2 years ago

Fortunately we made this a directive, so whatever change we make in the label screen will also work in the diary screen. First obvious fix is that the user inputs need to be directive specific instead of being common across directives. Also, for studies, we will never have the replaced mode.

So it almost seems like we want to have a set of two sets of options of the study/program - one with two trips and one with three trips and toggle between them (and their widths) depending on trip characteristics.

shankari commented 2 years ago

Toggling in response to user input was relatively straightforward - we just created a new displayInputDetails, changed the template to use it, and toggled it based on install type and mode. https://github.com/e-mission/e-mission-phone/pull/874/commits/c0876131a550943a1a832838f7f541eab0297025

However, toggling in response to load is going to be more complicated. We fill in the entries on initial load through mls.populateInputsAndInferences, which, in turn, is called inside $scope.$apply from infinite_scroll.js and list.js The service doesn't have access to the directive controller, so cannot swap the input details.

Couple of options: (1) set the input details into the trip, so it can be populated from the service. NOTE: in this case, we would need to move the switching code into the service as well (2) try to check the trip input in init and set the input details accordingly. NOTE: this won't work because we use collection-repeat, which pre-initializes the directives (3) watch for the $scope.trip to change and populate the input details accordingly

Let's try (3) briefly so that we can keep the complexity in the directive but fall back to (1) if it is too complex

shankari commented 2 years ago

$watch works great. Testing done: loaded with e-bike and without e-bike as the main mode and both worked as expected.

shankari commented 2 years ago

There's another minor issue that I just realized: we also need to fix the "To Label" filters. Otherwise, even if both the visible labels are filled out, the trip still stays in "to label".

shankari commented 2 years ago

This is also going to be complicated to fix. we need to fix both mls.updateVerifiability for the verify button, and recomputeDisplayTrips, neither of which is in the directive's controller. We're going to have to restructure completely to use the trip instead.

shankari commented 2 years ago

Tried a simple approach where we have two INPUT variables in the ConfirmHelper as well - INPUTS and BASE_INPUTS. We then toggle between them for each trip based on the install (program vs. study) and the mode. Running into an issue where we update the visibility right after we have initialized the trip (in mls.populateInputsAndInferences) and before we expanded it $watch callback of the directive.

This is the initial update (populateInputsAndInferences is only called from the list on startup), so let's try moving it into the $watch?

shankari commented 2 years ago

Ran into similar issue with $scope.recomputeDisplayTrips();, which is called directly from infinite_scroll.js. We need to move the expansion into the service, so it can be called from the service. And in turn, that means that we need to store the details also into the trip object.

This is really all controller stuff, so I hope we can refactor it when we unify diary and label screens.

shankari commented 2 years ago

Fixed the filtering in https://github.com/e-mission/e-mission-phone/pull/876

shankari commented 2 years ago

Another corner case fix to ensure that studies work too: https://github.com/e-mission/e-mission-phone/pull/878

shankari commented 2 years ago

I claim that this is done.