fix(Banner): isolate banner visibility using per-instance CSS variables #5751
+235
−200
Conversation
This file contains hidden or 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
solidusite
changed the title
commit: |
solidusite
force-pushed
the
issue-5014
branch
from
6b41343 to
7952404
Compare
solidusite
force-pushed
the
issue-5014
branch
from
7952404 to
4cbd574
Compare
Member
|
Thanks for the implementation @solidusite, would you mind fixing the conflicts? I had to make a fix for a security issue. |
Author
|
@benjamincanac done! I made a rebase |
Fixed issue where closing one banner would hide all banners on the page. Each banner now uses its own CSS custom property and pre-hydration script to prevent FOUC and manage visibility independently.
Updated the banner component to utilize `useId` for generating unique instance IDs, ensuring each banner has a distinct identifier. This change enhances the management of multiple banners on the page.
Updated the banner component to improve the logic for handling visibility persistence based on the presence of an ID prop. The titles of the banners inside playground have also been modified to clarify their behavior regarding closure and persistence.
Added a note emphasizing the necessity of specifying an `id` prop to maintain the dismissed state of the banner across page reloads. This update enhances user understanding of the banner's behavior regarding visibility persistence.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
Fixed issue where closing one banner would hide all banners on the page. Each banner now uses its own CSS custom property and pre-hydration script to prevent FOUC and manage visibility independently.
🔗 Linked issue
Resolves #5014
❓ Type of change
📚 Description
This PR fixes a bug where closing one banner would hide all banners on the page.
Problem
The previous implementation used a single global CSS class
hide-banneron the<html>element to control banner visibility. When a user closed any banner, this class was added to<html>, which would hide all banners with the.bannerclass instead of just the specific one that was closed.Solution
The fix introduces per-banner visibility management with improved persistence logic:
idprop or an auto-generated ID viauseId()idprop is provided. Banners without anidwill reappear on page reload.id, an inline script runs before Vue hydration to check localStorage and set the appropriate CSS custom property, preventing FOUC (Flash of Unstyled Content)--banner-{id}-display) to control visibility independentlyisVisibleref withv-showfor proper reactivity after hydrationdata-banner-idattribute to each banner for targeting specific instancesTechnical Details
data-v-{hash}attributes from rendered output)idprops📝 Checklist