Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

QMAPS-1883 refactor close buttons #1051

Merged
merged 35 commits into from
Apr 22, 2021
Merged

QMAPS-1883 refactor close buttons #1051

merged 35 commits into from
Apr 22, 2021

Conversation

xem
Copy link
Contributor

@xem xem commented Mar 31, 2021

Description

Done:

  • Mobile: remove close button on all panels except directions, menu, modal, alert, route details, favorites.
  • Desktop: remove close button on POI panel only
  • PoI Panel (PC/Mobile): whatever the navigation origin, put PoI name in search field and quit PoI panel when clicking close button
  • Mobile PoI panel: if we're coming from a PoI list or favorites list: show a return arrow next to search input that goes back to it
  • Put the return button logic in PanelManager
  • Put the search field filling logic in PanelManager
  • Make the back buttons work correctly (on PC and Mobile) even after we click on many PoIs in a row (history(-1) isn't enough)
  • Fill search input correctly (with the right timing) when a PoI is opened
  • Avoid transitions blinking when opening a poi from the list / returning to the list / clicking many PoIs in a row
  • Put "Favorites" in search field when favorites list is opened
  • Fix tests

DEMO:
https://www.loom.com/share/0d9a1bc709b0403197b8611b6016174e

TODO in another ticket?

  • Use the same return button on mobile for both closing suggest and returning to list?

@xem xem marked this pull request as draft March 31, 2021 15:06
@xem xem force-pushed the QMAPS-1883-close-buttons branch from b29acda to 248f018 Compare April 7, 2021 09:31
@xem xem marked this pull request as ready for review April 7, 2021 15:00
@xem xem requested review from bbecquet and G-Ray April 7, 2021 15:13
@bbecquet
Copy link
Contributor

bbecquet commented Apr 9, 2021

I've noticed some problems:

The transition effect in the top bar is repeated when you go from one POI to another.
Peek 09-04-2021 15-10

When in multiple POI mode (aka. category), the category name appears briefly in the field after being replaced by the clicked POI name
Peek 09-04-2021 15-14
This is because the search input change is not done in a centralized way like before but managed in the PanelManager and in the POIPanel.

BTW, all these search bar manipulations direclty in POIPanel are a serious step back from what we achieved some months ago, which was to centralize the management of its state in a single place to simplify and avoid consistency errors…
This is my fault, I should have pushed to merge #997 before, it would have enforced state centralization.
In the meantime, we must find a way here to manage as much things as we can outside POIPanel.

views/index.ejs Outdated Show resolved Hide resolved
src/panel/poi/PoiPanel.jsx Outdated Show resolved Hide resolved
src/panel/poi/PoiPanel.jsx Outdated Show resolved Hide resolved
@xem xem force-pushed the QMAPS-1883-close-buttons branch from e84270d to 3b5815f Compare April 12, 2021 07:52
@xem xem changed the title [DRAFT] QMAPS-1883 refactor close buttons QMAPS-1883 refactor close buttons Apr 12, 2021
@xem xem force-pushed the QMAPS-1883-close-buttons branch from 5d953d4 to 2d0e8a4 Compare April 14, 2021 14:52
@xem xem requested a review from bbecquet April 14, 2021 14:52
@xem xem marked this pull request as draft April 14, 2021 14:52
@xem xem force-pushed the QMAPS-1883-close-buttons branch from 10865c7 to c846ad2 Compare April 19, 2021 08:52
@xem xem marked this pull request as ready for review April 19, 2021 15:10
src/panel/PanelManager.jsx Outdated Show resolved Hide resolved
src/panel/PanelManager.jsx Outdated Show resolved Hide resolved
src/panel/PanelManager.jsx Outdated Show resolved Hide resolved
tests/integration/tests/autocomplete.js Outdated Show resolved Hide resolved
tests/integration/tests/autocomplete.js Outdated Show resolved Hide resolved
tests/integration/tests/autocomplete.js Outdated Show resolved Hide resolved
@xem xem requested review from amatissart and bbecquet April 21, 2021 09:39
@xem xem requested a review from amatissart April 21, 2021 16:11
@bbecquet bbecquet merged commit d37e7f8 into master Apr 22, 2021
@bbecquet bbecquet deleted the QMAPS-1883-close-buttons branch April 22, 2021 13:09
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants