Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Show all toasts #5933

Merged
merged 3 commits into from
Feb 28, 2024
Merged

Show all toasts #5933

merged 3 commits into from
Feb 28, 2024

Conversation

ktabors
Copy link
Member

@ktabors ktabors commented Feb 23, 2024

Closes

Do we want to add additional animations to this?

✅ Pull Request Checklist:

  • Included link to corresponding React Spectrum GitHub Issue.
  • Added/updated unit tests and storybook for this change (for new code or code which already has tests).
  • Filled out test instructions.
  • Updated documentation (if it already exists for this component).
  • Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

Goto a Toast story and add a Toast.

  • Toast should be added from the bottom.
  • Multiple toasts can be shown at the same time.
    • Toasts are sorted by order added.
  • Toast has enter and exit animations.
  • Toast should exit by timeout if defined or by close button (unless there is an action).

🧢 Your Project:

RSP

@rspbot
Copy link

rspbot commented Feb 23, 2024

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just one comment on the docs and a clarification question. I was surprised that we couldn't just keep everything the same on main and just set the priority of the toasts being added by the ToastContainer to the same value but I see you have changes here to flip it so that non prioritized toasts are added to the bottom instead of the top of the stack (hence your question at standup haha). Didn't realize it would involve changes in useToastState though, but it seems like an aria implementation with toasts with priorities set still work, though I guess a non-prioritized toast stack would now follow our bottom to top stack order now...

packages/@react-spectrum/toast/docs/Toast.mdx Outdated Show resolved Hide resolved

this.visibleToasts = prevToasts.concat(toasts).sort((a, b) => b.priority - a.priority);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So technically this updateVisibleToast function would work even if multiple toasts were being added/removed at the same time but now we use an index value so it assumes that we are only adding/removing one at a time right? I was a bit concerned but I guess the add/remove have always only supported adding one at a time anyways. Lemme know if I'm following these changes correctly

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The stately API currently exposes add, remove, close of one item at a time. When I add a whole bunch with five second timeouts quickly or try to close them all with the close button this change works.

The reason this code was modified is because it was moving closed and removed Toasts to the bottom of the visible queue then doing the exit animation. When you have one item showing and being removed that works, but with multiple visible items and one is removed, the removed item jumps to the bottom of the visible list. We need to keep the item being removed in the current position.

@rspbot
Copy link

rspbot commented Feb 27, 2024

Copy link
Member

@LFDanLu LFDanLu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Member

@reidbarber reidbarber left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM. Would be nice if we could have a transition on the toasts that are already in the stack, but we can handle that separately.

@ktabors
Copy link
Member Author

ktabors commented Feb 28, 2024

LGTM. Would be nice if we could have a transition on the toasts that are already in the stack, but we can handle that separately.

I expected requests for additional animations to come in via this PR review or in a testing session.

@rspbot
Copy link

rspbot commented Feb 28, 2024

@rspbot
Copy link

rspbot commented Feb 28, 2024

## API Changes

unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any', access: 'private' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown top level export { type: 'identifier', name: 'Column' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown type { type: 'link' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }
unknown top level export { type: 'any' }

@ktabors ktabors merged commit 707d26e into main Feb 28, 2024
27 checks passed
@ktabors ktabors deleted the toast_start_insert branch February 28, 2024 21:29
@@ -58,7 +58,7 @@ Then, queue a toast from anywhere:

Toasts are triggered using one of the methods of <TypeLink links={docs.links} type={docs.exports.ToastQueue} />. A &lt;<TypeLink links={docs.links} type={docs.exports.ToastContainer} />&gt; element must be rendered at the root of your app in order to display the queued toasts.

Toasts are shown according to a priority queue, depending on their variant. Actionable toasts are prioritized over non-actionable toasts, and errors are prioritized over other types of notifications. Only one toast is displayed at a time. See the [Spectrum design docs](https://spectrum.adobe.com/page/toast/#Priority-queue) for full information on toast priorities.
Toasts are shown according to the order they are added, with the most recent toast appearing at the bottom of the stack. Please use Toasts sparingly, see [Spectrum design docs](https://spectrum.corp.adobe.com/page/toast/#Too-many-toasts).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't put corp links in public docs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants