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

Fix JavaScript error when character count ID starts with a number #2080

Merged
merged 4 commits into from
Dec 22, 2020

Conversation

36degrees
Copy link
Contributor

As reported in #2058, the call to querySelector in character-count.js errors when the id starts with a number:

DOMException: Failed to execute 'querySelector' on 'Element': '[id=1_more-detail-info]' is not a valid selector.

Although it’s valid to start an ID with a number, the lack of quotes makes this an invalid CSS selector when interpolated with an ID that starts with a number, hence the error.

if (this.$textarea) {
this.$countMessage = $module.querySelector('[id=' + this.$textarea.id + '-info]')
}

This is because attribute values are either ident-tokens or string-tokens. Omitting the quotes means that the atrribute value is treated as an ident-token, which may not start with a number.

Quoting the value means it’s treated instead as a string-token, which can start with a number.

Ideally we’d be using document.getElementById to get the count message, but we’ve previously decided that would be a breaking change as it means the lookup is no longer scoped to the $module.

This also changes the way that we exclude hidden examples from the list of examples shown for each component, so that we still make their data available to the review app so that they can be viewed if you know the URL. This allows us to write JavaScript tests (which rely on examples in the review app) that target ‘edge cases’ without having to make the examples ‘visible’ in the review app.

Closes #2058

Exclude hidden examples from the list of examples shown for each component, whilst still making their data available to the review app so that they can be viewed if you know the URL.

This allows us to write JavaScript tests (which rely on examples in the review app) that target ‘edge cases’ without having to make the examples ‘visible’ in the review app.
This test fails with:

```
Character count › when JavaScript is available › when counting characters › when the ID starts with a number › still works correctly

    DOMException: Failed to execute 'querySelector' on 'Element': '[id=1_more-detail-info]' is not a valid selector.
```

which matches the console error reported in the issue [1]

[1]: #2058
Although it’s valid to start an ID with a number, the lack of quotes makes this an invalid CSS selector when interpolated with an ID that starts with a number, and an error is thrown:

> DOMException: Failed to execute 'querySelector' on 'Element': '[id=1_more-detail-info]' is not a valid selector.

This is because attribute values are either `ident-tokens` or `string-tokens` [1]. Omitting the quotes means that the atrribute value is treated as an ident-token, which may not start with a number [2].

Quoting the value means it’s treated instead as a string-token, which can start with a number [3].

Ideally we’d be using `document.getElementById` to get the count message, but we’ve previously decided that would be a breaking change as it means the lookup is no longer scoped to the $module [4].

[1]: https://www.w3.org/TR/selectors/#attribute-representation
[2]: https://www.w3.org/TR/css-syntax-3/#ident-token-diagram
[3]: https://www.w3.org/TR/css-syntax-3/#string-token-diagram
[4]: #1843 (comment)
@36degrees 36degrees changed the title Character count ids starting with numbers Fix error when character count ID starts with a number Dec 18, 2020
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-fronte-character--vmhobi December 18, 2020 13:51 Inactive
@36degrees 36degrees changed the title Fix error when character count ID starts with a number Fix JavaScript error when character count ID starts with a number Dec 18, 2020
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-fronte-character--vmhobi December 18, 2020 13:54 Inactive
Copy link
Contributor

@vanitabarrett vanitabarrett left a comment

Choose a reason for hiding this comment

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

Nice work on updating the hidden examples so we can use those 👍 Looks good to me!

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.

Character count fails if ID starts with a number.
3 participants