chore: Ensure class names conform to PascalCase #5716
Conversation
🦋 Changeset detectedLatest commit: 6ae6fbe The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
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 |
|
👋 Hi, this pull request contains changes to the source code that github/github depends on. If you are GitHub staff, we recommend testing these changes with github/github using the integration workflow. Thanks! |
github-actions
bot
added
the
integration-tests: recommended
| & > .ItemLabel { | ||
| font-weight: var(--base-text-weight-semibold); | ||
| } | ||
| } |
There was a problem hiding this comment.
@langermank I did not see any usage of this class anywhere, so I removed it. Can you confirm?
There was a problem hiding this comment.
Oh weird, it is used in Item
There was a problem hiding this comment.
Sorry, I meant ActionListContent--hasActiveSubItem. I didn't see an explicit reference to it, or a concatenation of hasActiveSubItem
There was a problem hiding this comment.
Ah! Yes, that doesn't appear to be used.
| [tabindex='0']:focus:not(:focus-visible):not(.focus-visible), | ||
| details-dialog:focus:not(:focus-visible):not(.focus-visible) { | ||
| [tabindex='0']:focus:not(:focus-visible):not(:global(.focus-visible)), | ||
| details-dialog:focus:not(:focus-visible):not(:global(.focus-visible)) { |
There was a problem hiding this comment.
@jonrohan I noticed that these did not have the :global wrapper, which I assume are needed for it to behave as expected.
size-limit report 📦
|
| &.\\:popover-open, | ||
| &.\\:popover-open::before { | ||
| /* stylelint-disable-next-line selector-class-pattern */ | ||
| &.\\:popover-open, &:global(.\\:popover-open)::before { |
There was a problem hiding this comment.
@jonrohan similar to https://github.com/primer/react/pull/5716/files#r1963911493, wrapped these in :global
github-actions
bot
requested a deployment
to
storybook-preview-5716
hussam-i-am
temporarily deployed
to
github-pages
GitHub Actions
Inactive
hussam-i-am
temporarily deployed
to
github-pages
GitHub Actions
Inactive
hussam-i-am
temporarily deployed
to
github-pages
GitHub Actions
Inactive
| })} | ||
| /> | ||
| ) : null} | ||
| {direction === SortDirection.DESC ? ( | ||
| <SortDescIcon | ||
| className={clsx('TableSortIcon', 'TableSortIcon--descending', { | ||
| [classes.TableSortIcon]: enabled, | ||
| [classes['TableSortIcon--descending']]: enabled, |
There was a problem hiding this comment.
For this style I think it'd be helpful to preserve the original convention from BEM until we can update it to use our preferred approach with data attributes. Having -descending ends up as a weird hybrid between the two and I think it can lose the modifier meaning because of it.
There was a problem hiding this comment.
Makes sense, updated
…am/stylelint-pascal
hussam-i-am
temporarily deployed
to
github-pages
GitHub Actions
Inactive
Closes https://github.com/github/primer/issues/4250
Changelog
New
Changed
selector-class-patternrule to check casing of class namesfocus-visibleand:popover-openclasses with:globaland make them exceptions to the ruleRemoved
Rollout strategy
Testing & Reviewing
Merge checklist