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(styling): Fix parent selectors in compat mode #2743

Conversation

NicholasBoll
Copy link
Member

@NicholasBoll NicholasBoll commented May 16, 2024

Summary

PR #2741 added the ability to target parent modifier class names to build selectors, but doesn't work in compat mode because compat mode merges all the CSS class names into a single, new class name. I added a parentModifiers function that handles this for both React Kit and CSS Kit and updated stencils to add inert class names for the purpose of always matching a selector.

I also fixed a bug where passing undefined to a Stencil for a modifier key resulted in overriding defaultModifiers.

Release Category

Styling


Before:

React static mode

<div {...mergeStyles(elemProps, myStencil({ size: 'large' }))} />

HTML

<div class="css-12345 css-abcde"></div>

Styles

.css-abcde :where(.css-123ab) {
  ...
}

React compat mode

<div {...mergeStyles(elemProps, [myStencil({ size: 'large' }), {maxWidth: 100}])} />

HTML

<div class="css-merged"></div>

Styles

.css-abcde :where(.css-123ab) {
  ...
}

Since compat mode merges class names together, the selector no longer works. This change forces the Stencil to add a class name of the modifier without the css- prefix. This still encodes needed information without the class name being removed by Emotion's cx function when style merging. The class names are inert directly (no selector matches), but are used by the parentModifier function.

After

React compat mode

<div {...mergeStyles(elemProps, [myStencil({ size: 'large' }), {maxWidth: 100}])} />

HTML

<div class="css-merged abcde"></div>

Styles

.abcde :where(.css-123ab) {
  ...
}

For the Reviewer

  • PR title is short and descriptive
  • PR summary describes the change (Fixes/Resolves linked correctly)

Where Should the Reviewer Start?

Tests

@NicholasBoll NicholasBoll added the ready for review Code is ready for review label May 16, 2024
Copy link

cypress bot commented May 16, 2024

Passing run #7303 ↗︎

0 970 3 0 Flakiness 0

Details:

Merge 7a4621c into 2313244...
Project: canvas-kit Commit: f1b62da53c ℹ️
Status: Passed Duration: 05:14 💡
Started: May 16, 2024 5:50 PM Ended: May 16, 2024 5:55 PM

Review all test suite changes for PR #2743 ↗︎

@NicholasBoll NicholasBoll added automerge and removed ready for review Code is ready for review labels May 16, 2024
@alanbsmith alanbsmith merged commit 78514b9 into Workday:prerelease/major May 16, 2024
19 checks passed
@NicholasBoll NicholasBoll deleted the fix/parent-modifier-selectors-compat-mode branch May 16, 2024 20:21
Copy link
Contributor

@josh-bagwell josh-bagwell left a comment

Choose a reason for hiding this comment

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

Approved.

Now I feel better

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants