Carousel container portal navigation #13162
Open
+122
−76
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
|
Hello 👋! When you're ready to run Chromatic, please apply the You will need to reapply the label each time you want to run Chromatic. |
|
Size Change: -657 B (-0.08%) Total Size: 870 kB
ℹ️ View Unchanged
|
github-actions
bot
removed
the
run_chromatic
Georges-GNM
reviewed
Jan 16, 2025
There was a problem hiding this comment.
Should we pass sectionId to scrollable/feature too?
There was a problem hiding this comment.
We don't as scrollable feature is only scrollable on mobile and mobile doesnt have navigation buttons, its just swipe controlled.
github-actions
bot
removed
the
run_chromatic
abeddow91
force-pushed
the
ab/carousel-portal-navigation
branch
from
61b622f to
5c72902
Compare
github-actions
bot
removed
the
run_chromatic
abeddow91
force-pushed
the
ab/carousel-portal-navigation
branch
from
e015978 to
5c72902
Compare
|
The |
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.
What does this change?
Refactors the carousel navigation buttons to use a React portal.
A portal allows rendering the button elements outside of the normal React component hierarchy, enabling flexibility in their placement within the DOM. This is particularly useful when the buttons need to be positioned outside the visual boundaries of the carousel component itself, such as on the fronts containers.
Why?
Depending on the breakpoint, the carousel front containers have their buttons placed outside of the component, in line with either the section heading or the left column. This had previously been achieved by using negative margins and absolute positioning to move the buttons outside of the container. This however, was quite brittle solution as it had to be calculate using the font size and line height of the container title and it also caused overlapping with the show/hide buttons as they occupied the same space.
By using portals, we can achieve a distinct position in the dom between the containers and there navigation buttons whilst still coupling the code in the carousel component.
The portal dynamically identifies a DOM element by constructing its ID using the
sectionIdprop and appends the suffix-carousel-navigation. This allows us to create distinct navigation portals per carousel.If the target DOM element is not found, a warning is logged in the console. The buttons will not be rendered if the portal target is unavailable.
As well as refactoring the nav buttons, this pr also hooks up show/hide with the beta containers and add a hidden class to the nav buttons so that they can also be hidden when a containers is closed.
Screenshots
Screen.Recording.2025-01-16.at.09.40.41.mov