bugfix(toast): raise the toast to the notification z-index tier - #1188
Conversation
Hello jeanmarcmilletscality,My role is to assist you with the merge of this Available options
Available commands
Status report is not available. |
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
5fddcf4 to
447b8b6
Compare
![claude[bot]](https://avatars.githubusercontent.com/in/1236702?s=60&v=4){kind=small}
| aria-labelledby={`${status}_toast`} | ||
| style={{ | ||
| position: 'fixed', | ||
| zIndex: zIndex.notification, |
There was a problem hiding this comment.
The PR description says toastAnchoring.ts is new and Toast.component.tsx gains a useContainWithinParent hook call plus a typed ref — but neither the file nor the hook call appear in this diff. The only component change here is adding zIndex.
Without useContainWithinParent, the toast does not automatically stay inside the parent's box. The ToastBesideAPanel story shows the fix requires the consumer to add contain: layout on the ancestor, which is a different contract from what the description promises. Either the file was not committed, or the description needs to be updated to match what the PR actually delivers.
JeanMarcMilletScality
changed the title
The toast rendered `position: fixed` with no `z-index` at all, so it painted in DOM order against whatever surrounded it. A toast raised from inside a modal was the visible consequence: modals render through a body portal at 8500, so the toast was painted underneath and never seen. The theme already reserves `notification` (9000) above that tier, and the sibling Notifications component already applies it — the toast was the only member of the family that did not. Anchoring is a separate matter and stays with the host. A fixed-position toast resolves its offsets against the viewport, which is not always the box the application was given, and only the element that owns that box can turn it into a containing block for fixed descendants (`contain: layout`). A component cannot reach its own ancestor, so this is recorded as a contract note in the component documentation rather than implemented here.
Two names for one component's tests (`Modal.test.tsx` and `Modal.component.test.tsx`) both pass CI, so nothing flags the split and the next contributor ends up testing the same component in two places. 32 of the 42 existing test files already mirror their source filename; write that down.
447b8b6 to
83cd73c
Compare
Waiting for approvalThe following approvals are needed before I can proceed with the merge:
Peer approvals must include at least 1 approval from the following list: |
|
/approve |
In the queueThe changeset has received all authorizations and has been added to the The changeset will be merged in:
There is no action required on your side. You will be notified here once IMPORTANT Please do not attempt to modify this pull request.
If you need this pull request to be removed from the queue, please contact a The following options are set: approve |
|
I have successfully merged the changeset of this pull request
Please check the status of the associated issue None. Goodbye jeanmarcmilletscality. |
3 participants
TL;DR
Toastrenderedposition: fixedwith noz-indexat all, so a toast raised from inside a modal was painted underneath it and never seen. It now uses the theme'snotificationtier, as its siblingNotificationscomponent already does.Context
Reported as a toast going missing when a host embeds an application in a box narrower than the viewport. Two separate defects were in play there, and only one belongs to this library.
Approach
z-indexat all — it painted in DOM order against a modal at8500position: fixedresolves against the viewport, which is not the box the application was givenThe second one cannot be fixed from inside
Toast. A fixed element resolves its offsets against its nearest containing-block ancestor, andcontain: layouton that ancestor is what redirects them — but a component cannot style its own ancestor, and no element in this library reliably sits at the application's boundary. Measured: in a 1440px viewport where the application's box ends at870, a toast attop: 3rem; right: 1remlands at1076→1426with nothing declared and at506→856once the box declarescontain: layout, inset preserved. That one declaration also coversNotifications, which has the sameposition: fixedand the same tier — another reason it belongs on the box rather than inside one component.So the requirement is documented instead:
stories/toast.stories.tsxgains a component-level note in the autodocs, stating that offsets resolve against the viewport and that whoever owns the box has to declarecontain: layout.Review focus
src/lib/components/toast/Toast.component.tsx›Toast— the toast now paints above modals (notification9000 >modal8500). Intended, but it is a visible ordering change for any flow that currently shows a modal over a toast.zIndexsits before thestylespread, so a caller-suppliedstylecan still override it.CLAUDE.md— records the existing one-test-file-per-module convention. No code impact.How to test
Toastwhile aModalis open. Before this change the toast was hidden behind the modal; now it paints above it.style={{ zIndex: … }}and confirm it still wins.npm run storybook→ Components / Feedback / Toast → the docs tab carries the anchoring note at the top.Follow-up
Anchoring is fixed where the box is owned, outside this library. This PR only stops the toast being painted under a modal, and records the contract.
What changed
Two lines in
Toast.component.tsx: thezIndeximport andzIndex: zIndex.notificationin the inline style. Eight lines instories/toast.stories.tsxfor the autodocs note — attached at component level rather than to a story, becauseToastWithProgressBarspreadsSimpleToastand a story-level note would have leaked onto it.Deliberately not here: no opt-out prop, since the tier already exists in the theme and the sibling component already uses it; no test, because asserting a
z-indexvalue would assert CSS rather than behaviour; and no story for the anchoring requirement — a demo would have to invent an application and a panel to show a consumer-side CSS declaration, which reads as though the component offered a containment mode.