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.

Sign up for GitHub

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

Jump to bottom

Update internal developer docs to use new configuration guidance #5516

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft

Update internal developer docs to use new configuration guidance #5516

wants to merge 2 commits into from

Conversation

domoscargin
Copy link
Contributor

Fixes #5501

Considerations

There is considerable overlap between these docs and the new docs on the website. I've mostly taken the approach of favouring the website, cutting out content and instead linking out to relevant bits of the site guidance, but the guidance here was really nice, and there are some bits that now feel a bit orphaned (ie: everything to do with Nunjucks).

I'd favour moving all the topics from this doc to the site and having a single documentation source.

@domoscargin
Update components coding standards
a76e6fe 
Include introduction to building JavaScript components.

Push support option to bottom of doc to encourage teams to build themselves.
@domoscargin domoscargin added documentation User requests new documentation or improvements to existing documentation javascript labels Nov 19, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Nov 19, 2024 •
edited
Loading

📋 Stats

File sizes

File Size
dist/govuk-frontend-development.min.css 118.41 KiB
dist/govuk-frontend-development.min.js 42.78 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 92.92 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 87.29 KiB
packages/govuk-frontend/dist/govuk/all.mjs 1.18 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 1.74 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.css 118.4 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 42.77 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.55 KiB
packages/govuk-frontend/dist/govuk/init.mjs 7.13 KiB

Modules

File Size (bundled) Size (minified)
all.mjs 82.76 KiB 40.37 KiB
accordion.mjs 26.62 KiB 13.41 KiB
button.mjs 9.13 KiB 3.78 KiB
character-count.mjs 25.43 KiB 10.9 KiB
checkboxes.mjs 7.81 KiB 3.42 KiB
error-summary.mjs 11.03 KiB 4.54 KiB
exit-this-page.mjs 20.24 KiB 10.34 KiB
header.mjs 6.46 KiB 3.22 KiB
notification-banner.mjs 9.39 KiB 3.7 KiB
password-input.mjs 18.28 KiB 8.33 KiB
radios.mjs 6.81 KiB 2.98 KiB
service-navigation.mjs 6.44 KiB 3.26 KiB
skip-link.mjs 6.4 KiB 2.76 KiB
tabs.mjs 12.04 KiB 6.67 KiB

View stats and visualisations on the review app

Action run for 1c0fcaa

romaricpascal
romaricpascal reviewed Nov 19, 2024
View reviewed changes
Copy link
Member

@romaricpascal romaricpascal left a comment

Choose a reason for hiding this comment

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

Strong linking game! Makes sense not to duplicate stuff we've already written on the Frontend Docs. I've added a couple of suggestions as I was reading the content, would be good to see what other people think as well (@querkmachine especially, as I think they were involved in the original piece of documentation).

docs/contributing/coding-standards/component-options.md Outdated Show resolved Hide resolved
docs/contributing/coding-standards/component-options.md Outdated Show resolved Hide resolved
docs/contributing/coding-standards/component-options.md Outdated Show resolved Hide resolved
docs/contributing/coding-standards/component-options.md Outdated Show resolved Hide resolved
docs/contributing/coding-standards/component-options.md Outdated
Comment on lines 33 to 34
class="my-component"
data-module="my-component"
Copy link
Member

Choose a reason for hiding this comment

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

question As these are internal docs, could we stick with the 'govuk' prefix? Maybe it's a matter of changing the introduction for the example, leading with something like:

'For example, this is how the Accordion does it for its rememberExpanded option:'

docs/contributing/coding-standards/component-options.md Outdated
{%- if params.rememberExpanded %} data-remember-expanded="{{ params.rememberExpanded | escape }}"{% endif %}>
...
</div>
```

The above code checks for the existence of the `rememberExpanded` parameter. If the parameter is present it adds the `data-remember-expanded` attribute. It then passes in the developer's defined value, running it through [Nunjucks' native `escape` filter](https://mozilla.github.io/nunjucks/templating.html#escape-aliased-as-e) to make sure that values can't break our HTML.

We can now call the Accordion's Nunjucks macro with our new `rememberExpanded` parameter:
We can now call the component's Nunjucks macro with our new `rememberExpanded` parameter:
Copy link
Member

Choose a reason for hiding this comment

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

issue The example following this statement still talks about the 'Accordion'.

If we switch the introduction of the previous example to present the Accordion's template as an example, we might be able to do something similar here:

Suggested change
We can now call the component's Nunjucks macro with our new `rememberExpanded` parameter:
With the previous code in the Accordion's template, this is how a page may set a specific value for the `rememberExpanded` option:

docs/contributing/coding-standards/component-options.md

## Adding a Nunjucks parameter

Most, but not all, components support adding arbitrary attributes and values through the `attributes` parameter. This method is also more verbose compared to having a dedicated Nunjucks parameter.
Copy link
Member

Choose a reason for hiding this comment

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

suggestion I think that bit is quite useful as it explains the two ways Nunjucks components can pass data-* attributes to the HTML for use by JavaScript components:

  • via an attributes parameter allowing arbitrary data-* attributes
  • via specific Nunjucks options that get converted to data-* attributes

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed!

docs/contributing/coding-standards/component-options.md Show resolved Hide resolved
docs/contributing/coding-standards/component-options.md Outdated
}
}
```
When passing configuration into your component's constructor, you can use nested objects to group options. This isn't possible when using data attributes, which only accept strings. Instead, you should follow our [naming conventions for data attributes](https://frontend.design-system.service.gov.uk/building-configurable-components/#receiving-configuration-from-data-attributes).
Copy link
Member

Choose a reason for hiding this comment

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

suggestion The "This isn't possible" reads as if you won't be able to set nested options through data attributes. Maybe we can spin this more positively with:

Suggested change
When passing configuration into your component's constructor, you can use nested objects to group options. This isn't possible when using data attributes, which only accept strings. Instead, you should follow our [naming conventions for data attributes](https://frontend.design-system.service.gov.uk/building-configurable-components/#receiving-configuration-from-data-attributes).
When passing configuration into your component's constructor, you can use nested objects to group options.
Data attributes only accept strings, but our [naming conventions for data attributes](https://frontend.design-system.service.gov.uk/building-configurable-components/#receiving-configuration-from-data-attributes) allow you to set options in nested objects using a dot `.` separator for each level of nesting. For example `data-i18n.showLabel="Show"` will set the same option as the following constructor argument:
```json
{
i18n: {
showLabel: 'Show'
}
}

docs/contributing/coding-standards/component-options.md

A common case is specifying whether a parameter accepts HTML or only plain text. For example, if a configuration option's value is inserted into the page using `innerText`, you might want to name the Nunjucks parameter something like `sectionLabelText` to indicate that HTML will not be parsed if provided.

There is more guidance on naming Nunjucks parameters in [the Nunjucks API documentation](https://github.com/alphagov/govuk-frontend/blob/main/docs/contributing/coding-standards/nunjucks-api.md#naming-options).

## Namespacing configuration options

You can group configuration options in JavaScript and HTML together by using namespaces; period-separated strings that prefix the configuration option name. Namespaces follow the same formats as other option names, being camelCase in JavaScript and kebab-case in HTML.

These are most commonly used for translation strings, which are usually namespaced under `i18n` (short for 'internationalisation').
Copy link
Member

Choose a reason for hiding this comment

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

suggestion Can we keep that as an explanation for the use of nested objects as it's the main one we have in govuk-frontend? Maybe to form a paragraph with "When passing configuration into your component's constructor...", moving the explanation on how to use data attributes to a second paragraph?

@domoscargin
Copy link
Contributor Author

@romaricpascal I've addressed all your comments - thanks!

I still think as a larger issue, ALL of this documentation should be in the frontend docs - it feels a bit arbitrary as to what's in these docs vs what's on the site. But that's a bigger job than the scope of this PR!

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

@romaricpascal romaricpascal romaricpascal left review comments

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
documentation User requests new documentation or improvements to existing documentation javascript
Projects
GOV.UK Design System cycle board
Status: Needs review 🔍
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Update internal docs on configuration options
3 participants
@domoscargin @romaricpascal @govuk-design-system-ci