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

Ensure GOV.UK Frontend component selectors cannot conflict when initialised #1443

Merged
merged 11 commits into from
Jun 13, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 71 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,77 @@

([PR #1376](https://github.com/alphagov/govuk-frontend/pull/1376))

- Ensure GOV.UK Frontend component selectors cannot conflict when initialised
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@m-green would be good to get your thoughts on this


Components which have JavaScript functionality include a data attribute called `[data-module]`,
we have added the `govuk-` prefix as a namespace.

This ensures the component's name does not conflict with other service level components
with the same name, which is consistent with our CSS class name convention.

If you're using Nunjucks macros, and are [using the `initAll` function](https://github.com/alphagov/govuk-frontend/blob/master/docs/installation/installing-with-npm.md#option-1-include-javascript) you will not be affected.

If you are using HTML, you will need to add the `govuk-` prefix to any `[data-module]` attributes.

For example the accordion component selector has changed from `[data-module="accordion"]` to `[data-module="govuk-accordion"]`.

```html
<div class="govuk-accordion" data-module="govuk-accordion">
<!-- ... -->
</div>
```

If you are initialising components manually, you will need to add the `govuk-` prefix to any CSS selectors used to find GOV.UK Frontend components:

```javascript
var $accordions = document.querySelector('[data-module="govuk-accordion"]')
```

([PR #1443](https://github.com/alphagov/govuk-frontend/pull/1443))


- Button and details components are now initialised consistently

The button and details components now use the `[data-module]` selector to initialise them.

If you're using Nunjucks macros, and are [using the `initAll` function](https://github.com/alphagov/govuk-frontend/blob/master/docs/installation/installing-with-npm.md#option-1-include-javascript) you will not be affected.

### Button component

The button component now is initialised with the `[data-module="govuk-button"]` selector instead of globally based on the document.

If you are using HTML, you need to add the `[data-module="govuk-button"]` attribute to your buttons:

```html
<button ... data-module="govuk-button">...</button>
```

If you are initialising the button component manually, you will need to reference this `[data-module]` attribute:

```javascript
var $buttons = document.querySelector('[data-module="govuk-button"]')
```

### Details component
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is there a way to avoid the duplication here? I duplicated it to be really clear but it makes it lengthy


The details component now is initialised with the `[data-module="govuk-details"]` selector instead of `details` globally.

If you are using the Nunjucks macro no changes is needed.

If you are using HTML, you need to add the `[data-module="govuk-details"]` attribute to your details:

```html
<details ... data-module="govuk-details">...</details>
```

If you are initialising the button component manually, you will need to reference this `[data-module]` attribute:

```javascript
var $details = document.querySelector('[data-module="govuk-details"]')
```

([PR #1443](https://github.com/alphagov/govuk-frontend/pull/1443))

- Pull Request Title goes here

Description goes here (optional)
Expand Down
40 changes: 20 additions & 20 deletions app/full-page-examples.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('Send your feedback')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -83,7 +83,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -100,7 +100,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('Have you changed your name?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -117,7 +117,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -134,7 +134,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('Passport details')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -151,7 +151,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -168,7 +168,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('Update your account details')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -185,7 +185,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -202,7 +202,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('Upload your photo')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -219,7 +219,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -236,7 +236,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('How do you want to sign in?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -253,7 +253,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -270,7 +270,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('What is your nationality?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -287,7 +287,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -304,7 +304,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('What is your address?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -321,7 +321,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand All @@ -338,7 +338,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('What is your home postcode?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -355,7 +355,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand Down Expand Up @@ -399,7 +399,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($.html()).toContain('What was the last country you visited?')

// Check that the error summary is not visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeFalsy()
done(err)
})
Expand All @@ -416,7 +416,7 @@ describe(`http://localhost:${PORT}/full-page-examples/`, () => {
expect($('title').text()).toContain('Error:')

// Check that the error summary is visible
let $errorSummary = $('[data-module="error-summary"]')
let $errorSummary = $('[data-module="govuk-error-summary"]')
expect($errorSummary.length).toBeTruthy()
done(err)
})
Expand Down
4 changes: 2 additions & 2 deletions docs/installation/installing-with-npm.md
Original file line number Diff line number Diff line change
Expand Up @@ -179,7 +179,7 @@ To initialise the first radio component on a page, use:

```js
var Radios = window.GOVUKFrontend.Radios
var $radio = document.querySelector('[data-module="radios"]')
var $radio = document.querySelector('[data-module="govuk-radios"]')
if ($radio) {
new Radios($radio).init()
}
Expand Down Expand Up @@ -229,7 +229,7 @@ You can use this attribute to initialise the component manually, this may be use
To initialise the first radio component on a page, use:

```js
var $radio = document.querySelector('[data-module="radios"]')
var $radio = document.querySelector('[data-module="govuk-radios"]')
if ($radio) {
new Radios($radio).init()
}
Expand Down
26 changes: 13 additions & 13 deletions src/all.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,45 +17,45 @@ function initAll (options) {
// Defaults to the entire document if nothing is set.
var scope = typeof options.scope !== 'undefined' ? options.scope : document

// Find all buttons with [role=button] on the scope to enhance.
new Button(scope).init()
var $buttons = scope.querySelectorAll('[data-module="govuk-button"]')
nodeListForEach($buttons, function ($button) {
new Button($button).init()
})

// Find all global accordion components to enhance.
var $accordions = scope.querySelectorAll('[data-module="accordion"]')
var $accordions = scope.querySelectorAll('[data-module="govuk-accordion"]')
nodeListForEach($accordions, function ($accordion) {
new Accordion($accordion).init()
})

// Find all global details elements to enhance.
var $details = scope.querySelectorAll('details')
var $details = scope.querySelectorAll('[data-module="govuk-details"]')
nodeListForEach($details, function ($detail) {
new Details($detail).init()
})

var $characterCount = scope.querySelectorAll('[data-module="character-count"]')
nodeListForEach($characterCount, function ($characterCount) {
var $characterCounts = scope.querySelectorAll('[data-module="govuk-character-count"]')
nodeListForEach($characterCounts, function ($characterCount) {
new CharacterCount($characterCount).init()
})

var $checkboxes = scope.querySelectorAll('[data-module="checkboxes"]')
var $checkboxes = scope.querySelectorAll('[data-module="govuk-checkboxes"]')
nodeListForEach($checkboxes, function ($checkbox) {
new Checkboxes($checkbox).init()
})

// Find first error summary module to enhance.
var $errorSummary = scope.querySelector('[data-module="error-summary"]')
var $errorSummary = scope.querySelector('[data-module="govuk-error-summary"]')
new ErrorSummary($errorSummary).init()

// Find first header module to enhance.
var $toggleButton = scope.querySelector('[data-module="header"]')
var $toggleButton = scope.querySelector('[data-module="govuk-header"]')
new Header($toggleButton).init()

var $radios = scope.querySelectorAll('[data-module="radios"]')
var $radios = scope.querySelectorAll('[data-module="govuk-radios"]')
nodeListForEach($radios, function ($radio) {
new Radios($radio).init()
})

var $tabs = scope.querySelectorAll('[data-module="tabs"]')
var $tabs = scope.querySelectorAll('[data-module="govuk-tabs"]')
nodeListForEach($tabs, function ($tabs) {
new Tabs($tabs).init()
})
Expand Down
2 changes: 1 addition & 1 deletion src/components/accordion/template.njk
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{% set id = params.id %}
{% set headingLevel = params.headingLevel if params.headingLevel else 2 %}

<div class="govuk-accordion {%- if params.classes %} {{ params.classes }}{% endif -%}" data-module="accordion" id="{{ id }}"
<div class="govuk-accordion {%- if params.classes %} {{ params.classes }}{% endif -%}" data-module="govuk-accordion" id="{{ id }}"
{%- for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}>
{% for item in params.items %}
<div class="govuk-accordion__section {% if item.expanded %}govuk-accordion__section--expanded{% endif %}">
Expand Down
18 changes: 6 additions & 12 deletions src/components/button/button.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,3 @@
/**
* JavaScript 'shim' to trigger the click event of element(s) when the space key is pressed.
*
* Created since some Assistive Technologies (for example some Screenreaders)
* will tell a user to press space on a 'button', so this functionality needs to be shimmed
* See https://github.com/alphagov/govuk_elements/pull/272#issuecomment-233028270
*
* Usage instructions:
* the 'shim' will be automatically initialised
*/
import '../../vendor/polyfills/Event' // addEventListener and event.target normaliziation

var KEY_SPACE = 32
Expand All @@ -19,8 +9,12 @@ function Button ($module) {
}

/**
* if the event target element has a role='button' and the event is key space pressed
* then it prevents the default event and triggers a click event
* JavaScript 'shim' to trigger the click event of element(s) when the space key is pressed.
*
* Created since some Assistive Technologies (for example some Screenreaders)
* will tell a user to press space on a 'button', so this functionality needs to be shimmed
* See https://github.com/alphagov/govuk_elements/pull/272#issuecomment-233028270
*
* @param {object} event event
*/
Button.prototype.handleKeyDown = function (event) {
Expand Down
2 changes: 1 addition & 1 deletion src/components/button/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ treat it as an interactive element - without this it will be

{#- Define common attributes that we can use across all element types #}

{%- set commonAttributes %} class="{{ classNames }}"{% for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}{% endset %}
{%- set commonAttributes %} class="{{ classNames }}" data-module="govuk-button"{% for attribute, value in params.attributes %} {{attribute}}="{{value}}"{% endfor %}{% endset %}

{#- Define common attributes we can use for both button and input types #}

Expand Down
2 changes: 1 addition & 1 deletion src/components/character-count/template.njk
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{% from "../textarea/macro.njk" import govukTextarea %}

<div class="govuk-character-count" data-module="character-count"
<div class="govuk-character-count" data-module="govuk-character-count"
{%- if params.maxlength %} data-maxlength="{{ params.maxlength }}"{% endif %}
{%- if params.threshold %} data-threshold="{{ params.threshold }}"{% endif %}
{%- if params.maxwords %} data-maxwords="{{ params.maxwords }}"{% endif %}>
Expand Down
2 changes: 1 addition & 1 deletion src/components/checkboxes/template.njk
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@
{% endif %}
<div class="govuk-checkboxes {%- if params.classes %} {{ params.classes }}{% endif %}"
{%- for attribute, value in params.attributes %} {{ attribute }}="{{ value }}"{% endfor %}
{%- if isConditional %} data-module="checkboxes"{% endif -%}>
{%- if isConditional %} data-module="govuk-checkboxes"{% endif -%}>
{% for item in params.items %}
{#- If the user explicitly sets an id, use this instead of the regular idPrefix -#}
{%- if item.id -%}
Expand Down
Loading