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

Deprecate Notice in favor of Alert #702

Open
henningmu opened this issue Sep 14, 2022 · 5 comments
Open

Deprecate Notice in favor of Alert #702

henningmu opened this issue Sep 14, 2022 · 5 comments

Comments

@henningmu
Copy link
Contributor

@gnapse I'm unclear on the difference between our Notice component and the Alert component. Shouldn't one be enough? Checking the Product Library Figma, there is also only the Alert and no concept of a Notice 🤔

@gnapse
Copy link
Contributor

gnapse commented Sep 14, 2022

This may be an issue about lack of documentation. The lack of a Figma design is a form of lack of documentation, but I'm referring to a lack of documentation in the code itself, in the storybook examples, for instance.

There are uses of <Notice /> in both products, and that's the ultimate "source of truth". It has new uses added as recently as when we worked on Todoist's 2FA. So ultimately, this is something our Design team should answer.

@Doist/design-product-hero do we really need the Notice component? What is its place in contrast with the Alert component?

Some comments on my side from what I think about having them both, and what their differences and similarities are:

  • They fulfill very similar purposes. Alert has more weight, and draws more attention than notice.
  • Alert makes it possible that the user can close it (if optionally supported, not all Alerts allow this). Notice does not offer this feature. A Notice only goes away when the condition that makes it appear goes away.
  • We already have some other UI elements that are visually similar to Notice, but in the context of a larger component. Most notable, and possibly the only example so far, is the message feature in our field components. This is not something that I say to support the existence of Notice. This is to say that maybe that's one acceptable use, but that all standalone instances of Notice could/should be replaced by Alert.

Examples

A couple of examples for how each component looks like:

Notice

Alert

CleanShot 2022-09-14 at 07 18 08@2x CleanShot 2022-09-14 at 07 22 25@2x

@henningmu
Copy link
Contributor Author

@gnapse this issue was motivated by work on this (internal) issue: https://github.com/Doist/Issues/issues/7617 – the new alert designs are more lightweight, making me question the usefulness of Notice even more 👍

@gnapse
Copy link
Contributor

gnapse commented Sep 14, 2022

Indeed, if the design team decides that we can replace all our uses of the standalone instances of <Notice /> with these new lighter alerts, then I'm ok with it. cc @Doist/design-product-hero

To be clear, it would not affect the notice-like messages embedded as part of the field component, but it would mean that we will no longer be able to render these borderless and background-less alerts1 as standalone.

If we're ok with that, then we deprecate it, and eventually remove it. The less we have to maintain, the better.

Footnotes

  1. (that's basically what notices are: borderless and background-less alerts that also cannot show any extra UI elements inside it such as buttons aligned to the right).

@anaf
Copy link

anaf commented Oct 20, 2022

I think that could work.
But before confirming it, do we have an easy way to find all the instances Notice, just so I can make sure?

@gnapse
Copy link
Contributor

gnapse commented May 29, 2023

@anaf sorry for the very late response. I was reviewing old issues in Reactist and realized this question slipped through the cracks. Here's a list of the current uses of <Notice /> in Todoist:

(1) Project export template as link: showing the error message.
(2) Reminders picker, showing error when date entered is invalid:
(3) Account settings, when enabling two-factor auth results in an error:
(4) Similar to the previous use case. If something goes wrong inside the two-factor auth settings.
(5) When an error occurs while trying to disconnect from an auth provider
(6) To show the error when trying to delete your account:
(7) Showing an error when it fails to add or update the payment method. Or when trying to remove a payment method
(Screenshot and context for that last one may be different, but it was not easily reproducible in my test account in the short term. Let me know if you really need it.)
image
(8) When entering an invalid email while inviting people to a workspace CleanShot 2023-05-29 at 16 36 12@2x

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

No branches or pull requests

3 participants