Skip to content

BGDIINF_SB-2785: Infobox does not hide menu anymore in phone mode#392

Merged
jedef merged 1 commit intodevelopfrom
feat-BGDIINF_SB-2785-scroll-not-possible-in-catalog-when-tooltip-open-mobile
Mar 28, 2023
Merged

BGDIINF_SB-2785: Infobox does not hide menu anymore in phone mode#392
jedef merged 1 commit intodevelopfrom
feat-BGDIINF_SB-2785-scroll-not-possible-in-catalog-when-tooltip-open-mobile

Conversation

@jedef
Copy link
Contributor

@jedef jedef commented Mar 14, 2023

It makes sense for the menu to be above the infobox in phone mode, as in phone mode the menu is fullscreen and so opens on top of everything else. In desktop mode on the contrary, the menu is open most of the time so a user should never be forced to close the menu just to see a popup. Thats why we leave the popup on top of the menu in desktop mode.

Test link

@jedef jedef requested a review from pakb March 14, 2023 11:12
@jedef jedef requested a review from ltshb March 27, 2023 08:45
Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

Change the title to match the bug that we fix, e.g. similar to the ticket title. For release note it is not clear why the menu is above infobox in phone mode.

Also when testing on desktop if the tooltip is in the map overlay it is behind the menu, but if it is set below it is on top of menu also blocking the menu interaction ! You cannot close the menu anymore ! I think this should be also fixed for desktop.

@jedef
Copy link
Contributor Author

jedef commented Mar 27, 2023

@ltshb For desktop we should maybe first discuss how we want to fix it because I do not see an obvious solution. Possible solutions:

  • Menu stops before the popup. => Hard to implement as the popup has a variable height and as the menu should still take the full height if no popup is open.
  • Popup does not take full width but starts at the right of the menu => Easier to implement and better than the previous option (as most desktop screens are in landscape mode), but maybe it will look strange. And if you close the menu, should the popup switch to take the full width again?
  • Menu is on top => This is illogical for me, as when you click on a feature you don't want its popup to be hidden by the menu. But for phone mode this is the logical behavior so that is what I changed in this PR.
  • Popup is on top => This is how it is now. I don't find it a too bad solution. If you want to access the menu, just close the popup or click the minus arrow to switch to a floating tooltip.

@jedef jedef changed the title BGDIINF_SB-2785: Menu is above infobox in phone mode BGDIINF_SB-2785: Infobox does not hide menu anymore in phone mode Mar 27, 2023
@jedef
Copy link
Contributor Author

jedef commented Mar 27, 2023

Change the title to match the bug that we fix, e.g. similar to the ticket title. For release note it is not clear why the menu is above infobox in phone mode.

Also when testing on desktop if the tooltip is in the map overlay it is behind the menu, but if it is set below it is on top of menu also blocking the menu interaction ! You cannot close the menu anymore ! I think this should be also fixed for desktop.

I added a more accurate description at the top to explain the changes and tried to change title to something a bit more understandable. However, the original ticket title is not an accurate description, as technically scrolling was still possible. It's just that a part of the menu was hidden by the popup.

Copy link
Contributor

@ltshb ltshb left a comment

Choose a reason for hiding this comment

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

ok as discussed in refinement meeting

Copy link
Contributor

@pakb pakb left a comment

Choose a reason for hiding this comment

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

As discussed together in refinement, this is a good enough solution and we will move on with this for the time being.

A future improvement would be to take the footer/infobox height into account when setting the height of the menu, but that requires extensive work (either render the infobox as pure CSS, and then adapt the placement with some CSS grid magic, or add the infobox height as Javascript computation in the menu height management)
We will wait for further negative feedback on this issue before spending any time on this complex topic

@jedef jedef force-pushed the feat-BGDIINF_SB-2785-scroll-not-possible-in-catalog-when-tooltip-open-mobile branch from 0fbfb42 to 4925c25 Compare March 28, 2023 09:34
It makes sense for the menu to be above the infobox in phone mode. In
desktop mode, we leave it as is for now.
@jedef jedef force-pushed the feat-BGDIINF_SB-2785-scroll-not-possible-in-catalog-when-tooltip-open-mobile branch from 4925c25 to 300972b Compare March 28, 2023 12:22
@jedef jedef merged commit 5714a82 into develop Mar 28, 2023
@jedef jedef deleted the feat-BGDIINF_SB-2785-scroll-not-possible-in-catalog-when-tooltip-open-mobile branch March 28, 2023 12:39
@pakb pakb mentioned this pull request May 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants