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

Review content of thrown errors #4072

Closed
3 tasks
Tracked by #3478
romaricpascal opened this issue Aug 11, 2023 · 5 comments · Fixed by #4351
Closed
3 tasks
Tracked by #3478

Review content of thrown errors #4072

romaricpascal opened this issue Aug 11, 2023 · 5 comments · Fixed by #4351
Assignees
Milestone

Comments

@romaricpascal
Copy link
Member

romaricpascal commented Aug 11, 2023

What

Review the wording of our error message and their names to make sure they're as useful as possible to our users

Why

The clearer these messages, the easier it will be for users for fix the errors that we raise.

Who needs to work on this

Developers, Tech writers

Who needs to review this

Developers, Tech writers

Done when

  • We've gathered all the messages of all errors we'll be throwing in v5
  • We've decided on the wording of the error messages
  • We've implemented any changes necessary in the codebase
@romaricpascal romaricpascal changed the title Review content of error messages Review content of thrown errors Aug 11, 2023
@romaricpascal romaricpascal added this to the v5.0 milestone Aug 11, 2023
@Vayras
Copy link

Vayras commented Aug 14, 2023

@romaricpascal I would like to contribute towards this issue.

@colinrotherham
Copy link
Contributor

colinrotherham commented Sep 7, 2023

Just a few per-developer things that have come up across error PRs so far

  1. Adding a component name suffix to error messages
  2. Explaining why something is wrong (selector, instance type, missed config key)
  3. Enclosing variable names in back ticks
  4. Enclosing strings in single quotes
  5. Enclosing strings in double quotes when they contain single quotes
  6. Using positive error names like NotSupportedError SupportError

Most important is to explain why something is wrong, where possible

@romaricpascal
Copy link
Member Author

romaricpascal commented Sep 22, 2023

Worth a quick look at how many of our components validate IDs set in aria-controls to ensure we're consistent in checking (or not checking) across the board.

@colinrotherham
Copy link
Contributor

colinrotherham commented Sep 26, 2023

Another note to say we've begun scoping using CSS selectors

Checkboxes: [data-module="govuk-checkboxes"] not found

throw new ElementError($module, {
  componentName: 'Checkboxes',
  identifier: `[data-module="${Checkboxes.moduleName}"]`
})

But others still use $module

throw new ElementError($module, {
  componentName: 'Exit this page',
  identifier: '$module'
})

Either way, do we need to worry when $module is overridden making our identifier incorrect?

@domoscargin
Copy link
Contributor

domoscargin commented Oct 17, 2023

Following on from this doc and some further discussions, we're aiming to format all our error messages with a mixture of plain English and selectors/variable names in backticks.

In general, this might look like:

<COMPONENT>: <PLAIN_ENGLISH_ELEMENT> (`<ELEMENT_IDENTIFIER>`) [<PLAIN_ENGLISH_ELEMENT_PROPERTY> (`<PROPERTY_IDENTIFIER`)] <ERROR>

Where:

COMPONENT:
Plain English component name (eg: "Character count")

PLAIN_ENGLISH_ELEMENT:
Plain English element that's causing the issue (eg: "Button")

ELEMENT_IDENTIFIER:
An identifier for the element in code. It could be a CSS selector, an HTML snippet, or a variable name. Whatever makes the most sense to provide to the end user.

PLAIN_ENGLISH_ELEMENT_PROPERTY (optional):
If the error is caused by a property of the element (such as an attribute), we can include this property in the message.

PROPERTY_IDENTIFIER (optional):
An identifier for the property in code. It could be a CSS selector, an HTML snippet, or a variable name. Whatever makes the most sense to provide to the end user.

However, in the case where something that doesn't fit this format makes more sense, we should just use that.

Examples

Checkboxes: Input elements (`<input type="checkbox">`) not found

Header: Button (`.govuk-js-header-toggle`) attribute (`aria-controls`) is not of type string

Character count: Form field (`.govuk-js-character-count`) is not of type `HTMLTextareaElement` or `HTMLInputElement`

Error summary: Root element (`$module`) not found

Tabs: Link elements (`a.govuk-tabs__tab`) not found

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

Successfully merging a pull request may close this issue.

4 participants