feat(Alert): add Alert component which uses @digdir/design-system-react

mikaelrss commented 11 months ago

Description

Added a new Alert component, with options for

severity - sets the style of the alert, and has four valid values "success" | "info" | "danger" | "warning".
title - sets the title via textResourceBindings.
description - sets the description via textResourceBindings.
useAsAlert - A boolean which specifies whether the component should have role="alert" or not. It should not have this for static elements displaying information, but should have it for dynamic elements that appear after a certain user input. I currently implemented the aria-live attribute with polite, but I think maybe this should be configurable. Or maybe it should be based on severity label. if useAsAlert is set, and severity is >= warning, aria-live should be set to assertive. This option is removed and rather determined automatically based on the hidden property of GenericComponent. If hidden is explicitly set to false we append role="alert" to the Alert component. There is however one problem with this. If an element is hidden with an expression, but the expression evaluates to false on page load, role="alert" will be appended to the Alert component on load. The same problem would exist for the old API, where the app developers used a useAsAlert flag.

Screenshot:

Related Issue(s)

closes #1284

Verification/QA

Manual functionality testing
- [x] I have tested these changes manually
- [ ] Creator of the original issue (or service owner) has been contacted for manual testing (or will be contacted when released in alpha)
- [ ] No testing done/necessary
Automated tests
- [x] Unit test(s) have been added/updated
- [x] Cypress E2E test(s) have been added/updated - I have added some alerts to frontend-tests so that the alert is tested with percy. I also updated some failing cypress tests for Soft validations. Stopped using the custom id-based selector, and used @testing-library/cypress selectors for testing the content of alert messages instead.
- [ ] No automatic tests are needed here (no functional changes/additions)
- [ ] I want someone to help me make some tests
UU/WCAG (follow these guidelines until we have our own)
- [x] I have tested with a screen reader/keyboard navigation/automated wcag validator - Tested with wave and orca screen reader. The screen reader reads alerts twice for some reason, but I think this is an orca issue. Could somebody test with VoiceOver on Mac?
- [ ] No testing done/necessary (no DOM/visual changes)
- [ ] I want someone to help me perform accessibility testing
User documentation @ altinn-studio-docs
- [x] Has been added/updated - Have submitted a PR with documentation here
- [ ] No functionality has been changed/added, so no documentation is needed
- [ ] I will do that later/have created an issue
Changes/additions to component properties
- [x] Changes are reflected in both src/layout/layout.d.ts and layout.schema.v1.json, and these are all backwards-compatible
- [ ] No changes made
Support in Altinn Studio
- [ ] Issue(s) created for support in Studio
- [ ] This change/feature does not require any changes to Altinn Studio
Sprint board
- [x] The original issue (or this PR itself) has been added to the Team Apps project and to the current sprint board
- [ ] I don't have permissions to do that, please help me out
Labels
- [x] I have added a kind/* label to this PR for proper release notes grouping
- [ ] I don't have permissions to add labels, please help me out

mikaelrss commented 11 months ago

I see that I might need to update this PR and split out the schema for AlertComponent into a separate schema once #1283 is merged.

olemartinorg commented 11 months ago

This has a striking resemblance to the Panel component we already have, although this looks a bit different. Be sure to document what's the difference between these two components, and what's the use-case for each of them.

Also, this reminds me of #1023, which we closed because we wanted to introduce validations using expressions instead via #726. Does this Alert component have to be a component, or the use-case really just the same as soft validations? In that case, I think we should probably consider updating the visuals for the soft validations instead of making a new component.

mikaelrss commented 11 months ago

This has a striking resemblance to the Panel component we already have, although this looks a bit different. Be sure to document what's the difference between these two components, and what's the use-case for each of them.

Also, this reminds me of #1023, which we closed because we wanted to introduce validations using expressions instead via #726. Does this Alert component have to be a component, or the use-case really just the same as soft validations? In that case, I think we should probably consider updating the visuals for the soft validations instead of making a new component.

Thanks for the feedback! Regarding #1023 @Febakke was involved in the startup meeting of this new component, and I think we said that this new component should take over the role of Panel in displaying messages which appear as some sort of user input. Please correct me if I am wrong here @Febakke

This component could also be used to display static messages which do not appear/disappear based on user input. Soft validations seem to me like they only appear as a validation message whenever the user provides some input.

Maybe we should have a chat about this component again @Febakke @olemartinorg @Magnusrm

olemartinorg commented 11 months ago

I see! 🤔

This component could also be used to display static messages which do not appear/disappear based on user input.

The useAsAlert flag contradicts that. It opens up for using this as something that works just like a validation message, but a validation message that circumvents all the other (current and future) functionality in validation messages. I would assume there comes a feature request at some point for "having the option to not let the user proceed to the next page in the form when a certain Alert is displayed" and at that point we're on our way to re-implement validation with new clothes.

Soft validations seem to me like they only appear as a validation message whenever the user provides some input.

Ah, yes - so Alert is, in a sense, permanent soft validations. 🤔 Again, the functionality overlap here makes me feel uneasy. I don't want to be difficult, and I fully understand why you'd want to use a "fully featured component" for these messages, as it's much more flexible than regular validations (and permanent, as you mention!), but I think we should discuss this some more before letting it out in the wild. I would be very confused as an app developer if we promoted 3 different methods that achieve very similar things.

Maybe implementing an 'initial' trigger for validations could be an alternative (worryingly, validation messages disappear right now if you refresh the app, and you won't get them back until you hit a trigger), or reworking the way triggers on validations work could be another option.

Tagging @RonnyB71 as well.

mikaelrss commented 11 months ago

The useAsAlert flag contradicts that. It opens up for using this as something that works just like a validation message, but a validation message that circumvents all the other (current and future) functionality in validation messages.

The thinking is that Alert can be used as both information and an alert that grabs the attention of a screen reader. This does not necessarily have to be a validation message, but can certainly be used as one also :sweat_smile:. It could very well just be additional information that is displayed when making a user action.

For instance, in our specific use case in DGM we want to display this piece of extra information to the users when they select "Ja, jeg har informert de andre etterlatte", even though this is not a validation message.

Ah, yes - so Alert is, in a sense, permanent soft validations.

Yes, Alert is both soft validations and permanent soft validations, and just information :sweat_smile:

In any case, it seems we need to discuss this component a little more in detail before we introduce it to the codebase.

Febakke commented 11 months ago

@olemartinorg I agree that this will be confusing for the app-developer and we have a lot of overlap here. I guess I did not think this trough when we had the initial meeting on this. 🐥 We should land on a plan on how to divide between alert and panel in a way that is easy to understand for our app-developers. Anyway, here are my thoughts on this. This is a suggestion on how it could work in an ideall world 😅

Alert Alert is a defined pattern, and we use the same icons and severity grades as NAV does in their component. Our goal should be to use alert in the same way inn Apps. It should only contain text, preferbly short texts. It should not be used for validating a form. But in the future it might be used for giving the user information about a technical fault that is not related to what the user has filled in. "We could not submit your form because our database is full, sorry"

NB: I see that NAV has changed their documentation to say that you should not give the alert component: role=alert. Im not sure why, if the alert is presented after the page has been loaded it must have this role for screen readers to catch it. (They do say that it should not be presented dynamicly)

Summary:

Should be used for important short messages for the user
Should have different severity levels
Should not have buttons or other components inside them
Can be static or presented based on something that has happened either from the user or something outside of their control

Panel When we first made our panel component, we had many different use cases and we thought it should handle all of them and we did at that time not think about concistensy across public solutions like NAV. This has been something that we have started to focus more on after starting to work on the design system.

The panel should be a grouping of elements that make it stand out from the rest of the page. It has no limitations on what it can contain. Example of uses, highlight parts of a paragraph you want to stand out or grouping form components with a text that is relevant.

The color should only be a stylistic choice, it should not inform the user of a grade of severity.

If it is presented to the user with dynamics it might need a live-region: polite depending on the content.

I have to 🏃, but there is also some overlap with soft validations that I have to think about in this context.

mikaelrss commented 11 months ago

After a discussion in todays daily meeting, we have decided that we will introduce the new Alert component and at the same time replace the visual design of Soft validations to use the new Alert component from @digdir/design-system-react