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

Fix the weird issue where the map popup breaks maps in the diary list #823

Merged
merged 11 commits into from
May 2, 2022

Conversation

shankari
Copy link
Contributor

By converting the popover into a separate view, similar to the detail view.
Presumably, this does something magical with the scope, which prevents the
unfortunate spillover of some kind of invalidation into the list view.

Issue reproduction:
e-mission/e-mission-docs#111 (comment)
e-mission/e-mission-docs#111 (comment)

Potential fixes (none of which worked):

Final solution:

  • Create a map of confirmed trip objects in the timeline
  • Reset it every time we modify the list of trips
  • Create a new simplified detail screen without sections
  • Look up the confirmed trip, convert it to geojson and display it
  • Copy over the id for the retrieved confirmed trips to ensure that we have a unique key
    Bonus fix: Handle an undefined input for the trip -> geojson function correctly

Testing done:

  • Loaded data for day in diary view
  • Clicked on detail from infinite scroll view
  • Went back to diary view
  • Everything was fine

By converting the popover into a separate view, similar to the detail view.
Presumably, this does something magical with the scope, which prevents the
unfortunate spillover of some kind of invalidation into the list view.

Issue reproduction:
e-mission/e-mission-docs#111 (comment)
e-mission/e-mission-docs#111 (comment)

Potential fixes (none of which worked):
- Removing invalidateSize: e-mission/e-mission-docs#111 (comment)
- Trying to make the map id depend on the index: e-mission/e-mission-docs#111 (comment)
- Changing from `collection-repeat` to `ng-repeat`: e-mission/e-mission-docs#111 (comment)
- pass in the trip object directly: e-mission/e-mission-docs#111 (comment)

Final solution:
- Create a map of confirmed trip objects in the timeline
- Reset it every time we modify the list of trips
- Create a new simplified detail screen without sections
- Look up the confirmed trip, convert it to geojson and display it
- Copy over the id for the retrieved confirmed trips to ensure that we have a unique key
Bonus fix: Handle an undefined input for the trip -> geojson function correctly

Testing done:
- Loaded data for day in diary view
- Clicked on detail from infinite scroll view
- Went back to diary view
- Everything was fine
So you don't have to see the trip and then go back to the list to verify
Fortunately, this was already a directive, so the change largely involved
adding the directive to the detail screen.

Couple of modifications:
- we allow the specification of a linkedid to make the DOM traversal simpler
- we choose the survey opt in the detail screen as well
- on `recomputeDisplayTrips`, we close the detail screen

TODO: Might want to configure the wait to recompute since 30 secs is too large
for the detail screen use case.

e-mission/e-mission-docs#111
This is a pretty straightforward extension to
7d98d5d
…secs

Summary of changes:
- create a new attribute called `recomputedelay`
- plumb it through `linkedsurvey` -> `multi-label-ui` directives
- set it to a default of thirty secs if it doesn't exist, and convert secs -> ms
- pass it through from the controller to the service
- use it in the service in place of the hardcoded value
Summary of changes:
- read in the filter checks for the survey option
- run all of them and go back to the list view if they are false (e.g. filter is false; would not be displayed)
- change the recompute delay to 5 secs for the detail screens so the screen disappears fairly quickly

Note that the check in the diary is fairly hacky since the trip doesn't have all the information required for the toLabelCheck and it always succeeds
Since setting them causes the maps in the list view to break:
e-mission/e-mission-docs#111 (comment)
e-mission/e-mission-docs#111 (comment)
Principled fix is being tracked in:
e-mission/e-mission-docs#726

Change:
- If we receive an invalidateSize when we are hidden (size == 0), then we
  ignore it instead of moving the view to match
Since an unspecified value returns an empty string ("") and is not undefined.
How that we have finally figured out and fixed the broken maps issue
Originally commented out due to:
e-mission/e-mission-docs#111 (comment)

That issue was fixed in 2b72468
(e-mission/e-mission-docs#111 (comment))

So we can now restore this useful fix

+ reduce the delay to help with impatient people :)
… screen and vice versa

Since the diary and label screens are not currently unified, values set in one
screen are not reflected in the other. This is confusing if people switch
between the two.

Hacking around this by copying the user inputs from the diary to the list and
vice versa after enter.

Changes:
- new function to copy input if newer
- add a write_ts to modified user inputs to track which is newer
- call the new function for all trips in the diary and for trips that are
  loaded in the label (since the label has more trips than the diary).
@shankari shankari merged commit dd64e8a into e-mission:master May 2, 2022
@shankari shankari deleted the fix_maps branch May 2, 2022 04:52
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.

1 participant