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 required fields to examples #1990

Merged
merged 28 commits into from
Oct 28, 2020
Merged

Conversation

vanitabarrett
Copy link
Contributor

@vanitabarrett vanitabarrett commented Oct 16, 2020

Closes #1965

What

  • Update component YAML examples to include required attributes for all examples
  • The only exception to the above is when we want to intentionally test how a component behaves with invalid data/in certain extremes, e.g: when no data is passed in at all, or when falsey values are provided.

Why

In a previous PR we reviewed the required attributes on all our component options to double check that they're correct (#1979). Following on from that work, we want to make sure the examples in the YAML files (used for tests and to generate macro-options.json and fixtures.json) properly reflect the required attributes.

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1990 October 16, 2020 15:46 Inactive
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.

The label component also seems to mark the for param as required, but most of the examples do not include it (and the template handles it being omitted) so possibly it shouldn't be marked as required?

@govuk-design-system-ci govuk-design-system-ci temporarily deployed to govuk-frontend-review-pr-1990 October 28, 2020 09:32 Inactive
@vanitabarrett
Copy link
Contributor Author

@36degrees Thanks for the review - I think I've addressed them all now in the last few commits. I've marked the label for attribute as not required (49c5308)

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.

I think this looks good to me. I haven't manually verified every single example - but until we've got a way to programatically validate all the examples I think we have to treat this as a 'best effort' exercise anyway.

@vanitabarrett vanitabarrett merged commit 63a3f54 into master Oct 28, 2020
@vanitabarrett vanitabarrett deleted the add-required-to-examples branch October 28, 2020 13:31
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 required component options to yaml examples
3 participants