Inline Errors Improvements

[tidy-dev]

Description

This PR creates some re-usuable components for when we want inline messages used to describe inputs. Errors, warnings, and captions. These are messages that should be associated by an aria-describedby that currently are not in GitHub Desktop (or not consistently).

This a POC towards implementing inline errors that we can use consistently across Desktop in our forms and that are accessible.

Accessibility design considerations:

1. The preferred approach is that the submit buttons on forms be enabled by default. When the user clicks on the form submission, validation occurs. Then, the focus is either moved to a flash banner at the top of a form if it exists and announced via role of alert, or to the first element that has a now visible inline error that is associated through aria-described and therefor announced.
2. In opposition to this, providing inline errors immediately on user input is an anti-pattern because when to announce then is unclear and can be interrupted by form navigation. Additionally, they can feel noisy as they are announced as a user traverses through a form.

GitHub Desktop Form submissions state:

1. Submit buttons are disabled until the user has provided input into all required fields.
2. Most form inputs have inline errors that provide immediate local validation as the user types.
3. We have dialog forms, welcome flow forms, and unique scenarios such as the commit summary form.

Current GitHub Desktop Messaging:

1. Error dialog banner, on submit -> server side attempt.
2. Error dialog banner, on user input
3. Inline warning, on user input that blocks form submission - technically an error.
4. Inline warning, on user input that does Not block form submission.
5. Disabled buttons with tooltips displaying the reason they are disabled (an error).
6. Error bubbles used in welcome flow similar to dialog banner shown after server side attempt but is displayed below the form.
7. Using the default "Required input" popover thing that is provided by chrome.

Lastly.. there are some other more one off messaging types like the warning near the commit button about protected branches, warnings about bidi characters, and warning about applying stashes. But, all those do not share a form submission process and I think could be treated and assessed in one off ways (tho could be reevaluated to see if there is any shared approach).

Notes about types of errors/warnings in GitHub Desktop.

1. Error dialog banner make sense for displaying server side errors that we do not know the input they associate with. This is sometimes called a "Flash" in primer docs.
2. The commit button uses a tooltip to explain why the button is disabled.. thus an error. This is an established pattern in GitHub Desktop and is an accessible approach (assuming opens on focus and read by screen readers). Changing this would likely be highly disruptive to current users.
3. We recognize that displaying local inline validation immediately with user input is an accessibility anti-pattern. However, we believe removing it would be a regression for our users. This POC use and aria-live in combination with input debouncing and adding non-visible characters to increase probability that the error is read. We could revisit this approach if/when we switch the form submission in dialogs to always have the submit button enabled so that the screen reader user is only bothered by the errors when they attempt to submit.

Some issues:

1. The dialog banner is used for local validation as a user types. These errors are not associated directly with the input that causes them, and since the banner has a role of alert, the error is only announced once and can be interrupted.
2. The dialog banner is not focused on form submission; thus, if the user wanted to tab to the inputs, it would not be the next focusable element.
3. The errors banner in the welcome flow should be at the top so that when focused and user wants to tab to inputs the form will be the next focusable element.
4. We have things displayed as warnings that are block the user from continuing form submission. Thus, should be errors.
5. Showing user input errors in the dialog banner when they are related to user input. If you tab off the input and then tab on, if they were inline, then an aria-described by could read that the error was related to the input.
6. Submit buttons are disabled by default.

In the above, there are two related issues going on and somewhat competing. Form submission buttons being disabled and how errors/warnings are displayed.

Currently, a change to this is not required for audit remediation. However, for the current tickets about the inline warnings that should be errors, the quick fix would be to throw them up in the dialog banner. But, that still leaves us with the aforementioned problems and is a little bit of regression. I am proposing that we become consistent with error/warning handling so we don't make a change now than then implement another one down the road when we are focusing on less immediate audit remediation.

Since enabling the submit forms is a much more disruptive and three are 42 instances of disabled buttons we would need to comb through. I am proposing that we don't change that approach now, but improve our errors.

Improving them by:
Inline Errors/local validation:

1. Make it so local validation that can be associated to a single input is displayed as an inline message immediately. Whether that is error or a warnings.
2. If a local validation blocks the user from continuing, style it as an error.
3. Since we do not have form submission to allow screen reader users to be directed to the error after the fact, this PR implements the aria-live on user input to drastically increase chance that a screen reader user will be made aware of the error or warning.
4. This implements an required id attribute on the error message component so that it can be properly associated with the input by aria-described by and will be announced on input focus.

Server side validation/not inline:
6. Separately to the inline work, make it so that the welcome flow and dialog banner/flashes look and behave consistently with focus management and being screen read on submission.

No change to commit button tooltip error management.

Questions.

1. If we do enable submit form buttons by default down the road, we will want to keep immediate displaying of inline local validation. Should we turn off the screen reading on input in favor of screen reading on form submission? If so, Is that ok since that is the ideal way to present errors to a screen reader user - not a regression of taking away immediate reading of the errors that visual users receive immediately?

.... probably will have more.. just trying to capture everything I currently have here.

Some reference material/discussion:

• https://primer.style/design/ui-patterns/forms/overview#inline-validation
• https://github.com/github/accessibility/blob/main/docs/deep-dive-notes/2022-01-19-form-validation.md#validation
• https://github.com/github/accessibility-audits/issues/4031
• https://github.com/github/accessibility-audits/issues/4052

To be continued...

1. If this approach is agreed upon :D, need to go through the other forms with such errors and update them to use this.
2. git warning also has a success message... could expand upon this to be able to provide success messages... or use in combination with the "caption" style.. which I got inspiration from the FormControl input of primer.
3. What it looks like? I kept warnings the same and used the color and icon of the dialog error for the errors to try to keep inline with Desktop Theme.. but completely open to change. I know primer bolds errors?
4. Should the "Would you like to <link>create a repository</link> instead? Be a caption not in error that is shown beneath the error in this particular use case?
5. If we use captions, should they have info icons by default and maybe conditionally be set to something else -> have a success icon if used to represent a success message, or have a protip/lightbulb is used as suggestion in above.
6. Add errors to our inputs in some way so that they are always tied to the input via aria-describedby similar to how our labels are automatically tied to our inputs? Primer React does it with having a FormControl component with FormControl.Validation children. https://primer.style/react/FormControl

[tidy-dev]

Took this to accessibility office hours today. As mentioned, this has accessibility anti patterns due to the submit buttons being disabled by default. Thus, with the acknowledgment that a fully accessible approach would include the submit buttons being fixed, this is a step in the right direction.

Side questions and answers:

• If we enable submit buttons down the road, is keep the inline errors appearing immediately ok?
  It is ok. The purpose the submit button is to ensure the errors are not missed.
• Should the debouncing aria-live be removed if submit button is added.
  Maybe. Definitely, if it was designed by the get go, then it would be better not to have the noisiness of constant
  reminder. But, it does not have to be removed, the purpose of the submit button is to ensure errors are not missed.
• Links in an error? like the “Create a repository” link.
  Not good practice. That should be redesigned so it is not part of the error. (But not a violation per say).

[sergiou87]

Code looks good and it works as expected!! ✨

[sergiou87]

✨ 🪄 ✨