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

Add support for localisation via data-* attributes to Accordion component #2818

Merged
merged 3 commits into from
Sep 1, 2022

Conversation

querkmachine
Copy link
Member

@querkmachine querkmachine commented Aug 30, 2022

Raising early as a draft PR for visibility and those sweet, sweet automated deployments. Will resolve #2699 on completion.

  • Adds the configuration merging function and modified Element.prototype.dataset polyfill from the config spike ([SPIKE][DO NOT MERGE] Create IE-compatible configuration merging function #2810).
  • Adds a new helper function, extractConfigByNamespace, which non-destructively filters a flattened configuration object by a given namespace (e.g. retrieving all key-value pairs where the key starts with 'i18n.')
  • Adds Nunjucks parameters for setting i18n related data-* attributes on the Accordion component and associated documentation.
  • Adds support for retrieving and merging configuration objects to the Accordion JS.
  • Adds an example of a localised Accordion component using data-* attributes.
  • Adds unit tests for the configuration merging and value extraction functions.
  • Adds integration tests for the Accordion component, checking that changes to the data-* attributes are reflected in the UI.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 August 30, 2022 17:13 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 August 31, 2022 10:57 Inactive
@querkmachine querkmachine self-assigned this Aug 31, 2022
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 August 31, 2022 11:30 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 August 31, 2022 15:38 Inactive
@querkmachine querkmachine requested a review from a team August 31, 2022 15:40
@querkmachine querkmachine marked this pull request as ready for review August 31, 2022 15:40
@querkmachine querkmachine changed the title [WIP] Add support for localisation via data-* attributes to Accordion component Add support for localisation via data-* attributes to Accordion component Aug 31, 2022
Copy link
Contributor

@domoscargin domoscargin left a comment

Choose a reason for hiding this comment

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

This looks good to me! A coupla incredibly pedantic requests, but non-blocking.

My only wish is that we could be writing shiny new ES6+ code straight away.

src/govuk/components/accordion/accordion.yaml Outdated Show resolved Hide resolved
src/govuk/components/accordion/accordion.yaml Outdated Show resolved Hide resolved
Copy link
Contributor

@36degrees 36degrees left a comment

Choose a reason for hiding this comment

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

A few very minor things but generally this is looking ace. Great work on the tests!

src/govuk/components/accordion/template.njk Show resolved Hide resolved
src/govuk/components/accordion/template.njk Outdated Show resolved Hide resolved
src/govuk/common.unit.test.mjs Outdated Show resolved Hide resolved
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 September 1, 2022 10:07 Inactive
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 September 1, 2022 10:49 Inactive
Vanita Barrett-Smith and others added 3 commits September 1, 2022 14:20
Have translated all strings that are currently possible to translate
(should be everything apart from strings in hardcoded JS)
This function and the modified Element.prototype.dataset polyfill are taken from an earlier spike (govuk-frontend PR #2810)

Add new helper function to extract config keys by namespace

This is used to pull out the i18n-related configuration keys, so that they can be passed into the I18n function.

Add data attribute localisation support to Accordion

Write unit tests for mergeConfigs and extractConfigByNamespace

Amend extractConfigByNamespace to pass unit tests

Add Nunjucks parameter documentation

Add integration tests for localisation of the accordion

Change test -> it

Add whitespace control to Yaml values

Tweaking whitespace

Add data attribute template tests
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-2818 September 1, 2022 13:23 Inactive
@querkmachine querkmachine merged commit 040db6d into main Sep 1, 2022
@querkmachine querkmachine deleted the kg-i18n-accordion-attributes branch September 1, 2022 13:29
@romaricpascal romaricpascal mentioned this pull request Nov 10, 2022
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.

Add ability to pass translation strings to accordion component via data-attributes
5 participants