Component-Specific CSS Selector Tests #1574

mxriverlynn · 2022-11-03T14:44:59Z

Description

This PR adds component-specific CSS selector tests to PVC. It looks at the css created for a specific component, and compares it to the CSS selectors that are produced by the various lookbook previews for the component. selectors that are not found in the immediate previews (not counting preview params) will fail the component-specific selector CI test and list the components that need additional previews.

some custom selectors are not applicable to this test and can be ignored by modifying the IGNORED_SELECTORS const in test/css/component_specific_selectors_test.rb. these often include selectors based on state, such as :hover or a specific aria tag.

Current Failures

This PR contains a few known failures in the CI step for testing selectors. This is caused by existing components with many selectors that either need to be ignored or for which previews need to be built. Because of the current volatility in these selectors and failures, the CI step for testing selectors is left as optional. This should allow the PR to land as-is, and let future PRs add the needed ignores and previews.

These components are currently known to be failing:

Primer::Alpha::ActionList
Primer::Beta::BlankSlate
Primer::Beta::Button
Primer::Beta::Label

Once these components are passing successfully, it's recommended the CI step for testing custom selectors be marked as required.

PR Task List:

adds tests that ensure all selectors defined in a component's CSS are used in previews
allow component-specific selector ignores, not just global patterns
group component options in lookbook previews, to ensure preview coverage of options
setup component-specific css selector tests as a separate, optional CI step

Component Preview Groups

There was a need to add many new previews that only differ by one param. Rather than exploding the list of items in the left hand navigation, preview groups are being added to the components where it makes sense.

For example, this is the new preview for Primer::Alpha::SegmentedControl, looking at the "Full Width" group:

The full list of components that have grouped previews, added in this PR:

Primer::Alpha::Banner
Primer::Alpha::SegmentedControl
Primer::Beta::Counter
Primer::Beta::ProgressBar

Integration

Does this change require any updates to code in production?

no. this primarily applies to CI builds of PVC while incidentally improving lookbook previews for some components

Merge checklist

Added/updated tests
Added/updated documentation
Added/updated previews

Run Component-Specific Selector tests

bundle exec rake test:component_css

Example Output from Failed Run:

❯ bundle exec rake test:component_css
Run options: --seed 13793

# Running:

...........................................................F

Failure:
ComponentSpecificSelectorsTest#test_all_selectors_are_previewed_for_primer_beta_blankslate [/Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:47]:
PVC Preview Class 'Primer::Beta::BlankslatePreview' does not render a preview for these selectors:

.blankslate code
.blankslate-capped
.blankslate-spacious
.blankslate-narrow
.blankslate-large img
.blankslate-large h3
.blankslate-large p
.blankslate-clean-background

Selectors without a preview may be ignored by updating 'IGNORED_SELECTORS' in /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb


rails test /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:29

.........F

Failure:
ComponentSpecificSelectorsTest#test_all_selectors_are_previewed_for_primer_beta_label [/Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:47]:
PVC Preview Class 'Primer::Beta::LabelPreview' does not render a preview for these selectors:

.labels
.label
.Label--large
[ ... ]
.Label--done
.Label--sponsors

Selectors without a preview may be ignored by updating 'IGNORED_SELECTORS' in /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb


rails test /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:29

....F

Failure:
ComponentSpecificSelectorsTest#test_all_selectors_are_previewed_for_primer_beta_button [/Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:47]:
PVC Preview Class 'Primer::Beta::ButtonPreview' does not render a preview for these selectors:


  .Button[aria-disabled='true']
.Button-content--alignStart
.Button--small
.Button--small .Button-label
[ ... ]
.Button--iconOnly.Button--large

Selectors without a preview may be ignored by updating 'IGNORED_SELECTORS' in /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb


rails test /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:29

...F

Failure:
ComponentSpecificSelectorsTest#test_all_selectors_are_previewed_for_primer_alpha_actionlist [/Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:47]:
PVC Preview Class 'Primer::Alpha::ActionListPreview' does not render a preview for these selectors:

.ActionListWrap--inset
.ActionListItem[hidden] + .ActionList-sectionDivider
.ActionListItem.ActionListItem--hasSubItem > .ActionListContent
.ActionListItem[aria-checked='true'] .ActionListItem-multiSelectCheckmark
.ActionListItem-action--trailing
[ ... ]
.ActionListItem-action
.ActionListItem--subItem > .ActionListContent > .ActionListItem-label
.ActionList-sectionDivider--filled

Selectors without a preview may be ignored by updating 'IGNORED_SELECTORS' in /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb


rails test /Users/riverlynn/dev/primer/view_components/test/css/component_specific_selectors_test.rb:29

...........

Finished in 6.671226s, 13.4908 runs/s, 5.9959 assertions/s.
90 runs, 40 assertions, 4 failures, 0 errors, 0 skips

…test

changeset-bot · 2022-11-03T14:45:02Z

🦋 Changeset detected

Latest commit: fa62e91

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@primer/view-components	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

… for each

…-css-selector-tests

…ault, less useful message

jonrohan · 2022-11-03T16:35:00Z

adds tests that ensure the CSS selectors used by a component are valid

Let's do this one in a separate PR

…s, by component class

jonrohan · 2022-11-10T17:18:42Z

.github/workflows/test_css.yml

+  cancel-in-progress: true
+
+jobs:
+  selectors:


Could we move this to the test.yml file?

.github/workflows/test_css.yml

jonrohan · 2022-11-10T17:20:42Z

.github/workflows/test_css.yml

+    name: Component-specific selectors
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false


Not sure the fail-fast is doing anything here outside of the matrix context

Co-authored-by: Jon Rohan <rohan@github.com>

…-css-selector-tests

github-actions · 2022-11-10T18:54:19Z

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

…or-tests' into mxriverlynn/component-css-selector-tests

jonrohan

Looks great!

… test failure

mxriverlynn · 2022-11-10T20:42:11Z

app/components/primer/beta/button.pcss

-  /* &:focus {
-    @mixin focusOutline;
-  } */


with this commented out, PCSS copies it as a literal comment with no parsing into the final css output. this was causing test/components/component_css_test.rb to fail in the test suite, because that test looks for the presence of any & regardless of context

jonrohan

Ship it

mxriverlynn added 2 commits November 3, 2022 09:28

added first pass at making a working css selector test for components

03537e3

working on a bit more code organization and error message for failed …

2cb6eca

…test

mxriverlynn temporarily deployed to review-pr-1574 November 3, 2022 14:45 Inactive

mxriverlynn temporarily deployed to github-pages November 3, 2022 14:49 Inactive

mxriverlynn added 3 commits November 3, 2022 10:30

loop through all the components with previews, and test the selectors…

fee5bf6

… for each

correcting the test method name generation

500f9a6

Merge remote-tracking branch 'origin/main' into mxriverlynn/component…

4586618

…-css-selector-tests

mxriverlynn had a problem deploying to review-pr-1574 November 3, 2022 15:32 Error

mxriverlynn temporarily deployed to review-pr-1574 November 3, 2022 15:33 Inactive

mxriverlynn temporarily deployed to github-pages November 3, 2022 15:38 Inactive

mxriverlynn added 2 commits November 3, 2022 10:40

better failure message and code to generate it

558a8c8

improving assertion so it doesn't muddy failure messages with the def…

f0f2e28

…ault, less useful message

mxriverlynn had a problem deploying to review-pr-1574 November 3, 2022 15:45 Error

further improvements to the failure message

59773f3

mxriverlynn had a problem deploying to review-pr-1574 November 3, 2022 15:50 Error

ignored 'to' selector

23576d5

mxriverlynn temporarily deployed to review-pr-1574 November 3, 2022 15:52 Inactive

mxriverlynn temporarily deployed to github-pages November 3, 2022 15:56 Inactive

rubocop cleanup

63dea07

mxriverlynn temporarily deployed to review-pr-1574 November 3, 2022 15:59 Inactive

code cleanup for remaining selectors

fde18ee

mxriverlynn temporarily deployed to github-pages November 3, 2022 16:03 Inactive

mxriverlynn temporarily deployed to review-pr-1574 November 3, 2022 16:12 Inactive

mxriverlynn temporarily deployed to github-pages November 3, 2022 16:16 Inactive

jonrohan added the skip changeset Pull requests that don't change the library output label Nov 3, 2022

update ignored_selectors to allow global vs component specific ignore…

b517060

…s, by component class

mxriverlynn had a problem deploying to review-pr-1574 November 3, 2022 18:27 Error

mxriverlynn temporarily deployed to github-pages November 3, 2022 18:31 Inactive

mxriverlynn requested review from camertron and jonrohan November 10, 2022 16:34

jonrohan reviewed Nov 10, 2022

View reviewed changes

Update .github/workflows/test_css.yml

36d2b19

Co-authored-by: Jon Rohan <rohan@github.com>

mxriverlynn had a problem deploying to review-pr-1574 November 10, 2022 18:50 Error

Merge remote-tracking branch 'origin/main' into mxriverlynn/component…

50f9c4f

…-css-selector-tests

mxriverlynn had a problem deploying to review-pr-1574 November 10, 2022 18:51 Error

mxriverlynn and others added 2 commits November 10, 2022 12:53

move css tests into test.yml

d59febe

Generating component snapshots

fa3a4de

Merge remote-tracking branch 'origin/mxriverlynn/component-css-select…

c2d1677

…or-tests' into mxriverlynn/component-css-selector-tests

mxriverlynn temporarily deployed to review-pr-1574 November 10, 2022 18:55 Inactive

jonrohan approved these changes Nov 10, 2022

View reviewed changes

mxriverlynn temporarily deployed to github-pages November 10, 2022 19:00 Inactive

correcting preview rendering for segmented control tests

2abdde5

mxriverlynn deployed to review-pr-1574 November 10, 2022 20:13 View deployment

mxriverlynn temporarily deployed to github-pages November 10, 2022 20:17 Inactive

removing comment that included an ampersand, because it was causing a…

49ed298

… test failure

mxriverlynn had a problem deploying to review-pr-1574 November 10, 2022 20:34 Error

mxriverlynn temporarily deployed to github-pages November 10, 2022 20:38 Inactive

reset folder what's on main

91187ee

mxriverlynn had a problem deploying to review-pr-1574 November 10, 2022 20:41 Failure

mxriverlynn commented Nov 10, 2022

View reviewed changes

Generating component snapshots

fa62e91

jonrohan approved these changes Nov 10, 2022

View reviewed changes

jonrohan merged commit b3e351d into main Nov 10, 2022

jonrohan deleted the mxriverlynn/component-css-selector-tests branch November 10, 2022 21:07

primer-css mentioned this pull request Nov 10, 2022

Release Tracking #1582

Merged

simurai mentioned this pull request Nov 14, 2022

Update css selector test #1602

Merged

5 tasks

mxriverlynn mentioned this pull request Nov 17, 2022

Tests to validate CSS used by components #1608

Merged

3 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Component-Specific CSS Selector Tests #1574

Component-Specific CSS Selector Tests #1574

mxriverlynn commented Nov 3, 2022 •

edited

Loading

changeset-bot bot commented Nov 3, 2022 •

edited

Loading

jonrohan commented Nov 3, 2022

jonrohan Nov 10, 2022

jonrohan Nov 10, 2022

github-actions bot commented Nov 10, 2022 •

edited

Loading

jonrohan left a comment

mxriverlynn Nov 10, 2022

jonrohan left a comment

Component-Specific CSS Selector Tests #1574

Component-Specific CSS Selector Tests #1574

Conversation

mxriverlynn commented Nov 3, 2022 • edited Loading

Description

Current Failures

PR Task List:

Component Preview Groups

Integration

Merge checklist

Run Component-Specific Selector tests

Example Output from Failed Run:

changeset-bot bot commented Nov 3, 2022 • edited Loading

🦋 Changeset detected

jonrohan commented Nov 3, 2022

jonrohan Nov 10, 2022

Choose a reason for hiding this comment

jonrohan Nov 10, 2022

Choose a reason for hiding this comment

github-actions bot commented Nov 10, 2022 • edited Loading

⚠️ Visual differences found

jonrohan left a comment

Choose a reason for hiding this comment

mxriverlynn Nov 10, 2022

Choose a reason for hiding this comment

jonrohan left a comment

Choose a reason for hiding this comment

mxriverlynn commented Nov 3, 2022 •

edited

Loading

changeset-bot bot commented Nov 3, 2022 •

edited

Loading

github-actions bot commented Nov 10, 2022 •

edited

Loading