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 Review app code examples using example.options #4202

Merged
merged 6 commits into from
Sep 19, 2023

Conversation

colinrotherham
Copy link
Contributor

Fixes Markup and Macro code rendering in the review app

Macro options were renamed to example.data example.options in 7a2d138 via #4042

We missed this because the {% for example in componentFixtures.fixtures %} loop was in a separate file

Code rendering has missing component options

To help avoid Nunjucks issues like this in future I've included a few changes such as:

  1. Move code partial into showExamples.njk for easier debugging
  2. Configure Nunjucks with dev: true to log stack traces etc

@colinrotherham colinrotherham added the 🐛 bug Something isn't working the way it should (including incorrect wording in documentation) label Sep 11, 2023
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4202 September 11, 2023 15:50 Inactive
@colinrotherham
Copy link
Contributor Author

@romaricpascal This change was missed from 7a2d138 🤦‍♂️

@github-actions
Copy link

github-actions bot commented Sep 11, 2023

📋 Stats

File sizes

File Size
dist/govuk-frontend-4.6.0.min.css 118.07 KiB
dist/govuk-frontend-4.6.0.min.js 47.1 KiB
dist/govuk-frontend-ie8-4.6.0.min.css 79.27 KiB
packages/govuk-frontend/dist/govuk/all.bundle.js 75.84 KiB
packages/govuk-frontend/dist/govuk/all.bundle.mjs 71.24 KiB
packages/govuk-frontend/dist/govuk/all.mjs 3.84 KiB
packages/govuk-frontend/dist/govuk/govuk-frontend-component.mjs 359 B
packages/govuk-frontend/dist/govuk/govuk-frontend.min.js 36.79 KiB
packages/govuk-frontend/dist/govuk/i18n.mjs 5.83 KiB

Modules

File Size
all.mjs 67.34 KiB
components/accordion/accordion.mjs 21.9 KiB
components/button/button.mjs 4.71 KiB
components/character-count/character-count.mjs 21.64 KiB
components/checkboxes/checkboxes.mjs 5.41 KiB
components/error-summary/error-summary.mjs 6.01 KiB
components/exit-this-page/exit-this-page.mjs 16.6 KiB
components/header/header.mjs 3.34 KiB
components/notification-banner/notification-banner.mjs 4.54 KiB
components/radios/radios.mjs 4.41 KiB
components/skip-link/skip-link.mjs 3.73 KiB
components/tabs/tabs.mjs 9.07 KiB

View stats and visualisations on the review app


Action run for e29ffd9

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.

Whoops for missing that at the time, didn't occur to me when reviewing to check for leftover offcurences of example.data or even \.data(?!set) (many .dataset that are not relevant).

Cheers for tidying it up! Don't think there's much risk on moving the code partial inside the macro, unless anyone in @alphagov/design-system-developers was planning to reuse that bit specifically?

@@ -20,6 +20,7 @@ export function renderer(app) {
const env = nunjucksEnv(
[join(paths.app, 'src/views')],
{
dev: true, // log stack traces
Copy link
Member

Choose a reason for hiding this comment

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

Ohhh, that's going to be handy for debugging! 🥳

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I discovered it recently and I'm getting bored of adding it 😆

You get to see Nunjucks stack traces right into global/filter code too

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4202 September 13, 2023 11:05 Inactive
We missed a change from `example.data` to `example.options` since the `{% for example in componentFixtures.fixtures %}` loop was in a separate file
The dependency `chokidar` is no longer needed for production
Minor speed up for review app performance
@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-pr-4202 September 19, 2023 16:09 Inactive
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.

Plenty of time to raise concerns about the code.njk disappearance, nothing raised, let's merge! 🥳

@colinrotherham colinrotherham merged commit ee82fd3 into main Sep 19, 2023
@colinrotherham colinrotherham deleted the fix-preview-code branch September 19, 2023 16:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working the way it should (including incorrect wording in documentation)
Projects
Development

Successfully merging this pull request may close these issues.

3 participants