[FW][FIX] website: make anchor links work in offcanvas mobile menu #146897
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
A anchor menu in the mobile collapse related to an element in the current page doesn't work correctly: - The collapse-menu doesn't close - If the menu is large enough, on mobile, you actually don't even see the page scrolling behind it so it looks like the click did nothing. This is probably because of the switch from BS4 to BS5. In later version, it's even worse, as we are using bootstrap Offcanvas instead of collapse, creating another bug: - The page doesn't scroll at all This will be fixed in the forward port. This commit simply manually closes the collapse before starting our own smooth scrolling. It also targets the desktop offcanvas menu (when hamburger layout is selected). opw-3604963 X-original-commit: db881e6
|
|
|
This PR targets saas-16.3 and is part of the forward-port chain. Further PRs will be created up to master. More info at https://github.com/odoo/odoo/wiki/Mergebot#forward-port |
robodoo
pushed a commit
that referenced
this pull request
Dec 20, 2023
A anchor menu in the mobile collapse related to an element in the current page doesn't work correctly: - The collapse-menu doesn't close - If the menu is large enough, on mobile, you actually don't even see the page scrolling behind it so it looks like the click did nothing. This is probably because of the switch from BS4 to BS5. In later version, it's even worse, as we are using bootstrap Offcanvas instead of collapse, creating another bug: - The page doesn't scroll at all This will be fixed in the forward port. This commit simply manually closes the collapse before starting our own smooth scrolling. It also targets the desktop offcanvas menu (when hamburger layout is selected). opw-3604963 closes #146897 X-original-commit: db881e6 Signed-off-by: Quentin Smetz (qsm) <[email protected]> Signed-off-by: Romain Derie (rde) <[email protected]>
fw-bot
deleted the
saas-16.3-17.0-fix-offcanvas-mobile-anchor-rde-lCBq-fw
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
A anchor menu in the mobile offcanvas related to an element in the
current page doesn't work in offcanvas.
This is because the menu anchor navigation is hooked to use our own
scrolling behavior instead of the browser one.
Doing so, we preventDefault, which prevent the offcanvas menu to close
itself when clicking on a anchor menu.
This commit simply manually close the offcanvas and once the closing
animation is complete, starts our own smooth scrolling.
Another possibility would have been to just close manually the offcanvas
without a preventDefault and without a call to our custom scrolling
method.
Doing so, the browser would naturally scroll to the element while we
close the offcanvas but it would be less elegant as you wouldn't see the
scrolling animation.
Note that the offcanvas was introduced with commit 1.
opw-3604963
Forward-Port-Of: #146132