Carousel container portal navigation #13162
Merged
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: -541 B (-0.06%) Total Size: 884 kB
ℹ️ View Unchanged
|
github-actions
bot
removed
the
run_chromatic
Georges-GNM
reviewed
Jan 16, 2025
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 |
abeddow91
force-pushed
the
ab/carousel-portal-navigation
branch
from
5c72902 to
761702e
Compare
abeddow91
commented
Jan 21, 2025
abeddow91
commented
Jan 21, 2025
github-actions
bot
removed
the
run_chromatic
Fweddi
reviewed
Jan 22, 2025
Fweddi
reviewed
Jan 22, 2025
Fweddi
reviewed
Jan 22, 2025
Fweddi
approved these changes
Jan 22, 2025
|
Seen on PROD (merged by @abeddow91 1 hour, 13 minutes and 30 seconds ago) Please check your changes! |
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.
Why Portals?
As highlighted above, the design requirements necessitate that the navigation buttons for the carousel be placed outside the boundaries of the carousel component itself. This positioning ensures alignment with external elements such as section headings or adjacent columns, depending on the screen breakpoint. To implement this, we considered three potential approaches:
1. Negative margins
Initially, negative margins were used to manually adjust the position of the buttons relative to the carousel container. However, this solution was brittle and prone to issues:
Overall, this approach was fragile and difficult to scale or adapt to new requirements.
2. Lifting State to a Shared Parent Component
Another option was to lift the state to a parent component shared by both the carousel and its navigation buttons. This approach offered some advantages, such as:
However, this approach came with significant trade-offs:
3. Portals
React Portals emerged as the ideal solution because they allow rendering the navigation buttons in a separate part of the DOM while maintaining their state and logic within the carousel component. This approach offers several benefits:
Unfortunately, portals are inherently rendered on the client side and cannot be server-side rendered, which can result in some layout shifting. However, this issue has been largely mitigated by introducing a fixed minimum height placeholder to reserve space for the navigation buttons during initial rendering.
Screenshots
Screen.Recording.2025-01-16.at.09.40.41.mov