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

refactor(styles): set CSS vars in pin mixin #1296

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

Conversation

ogonkov
Copy link
Contributor

@ogonkov ogonkov commented Jan 31, 2024

This will set CSS-vars in private namespace, that will allow to developer reuse border-radius in children elements, for example to match nested element border-radius by reducing it via calc().

@gravity-ui-bot
Copy link
Contributor

Playwright Test Component is ready.

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

styles/mixins.scss Show resolved Hide resolved
styles/mixins.scss Show resolved Hide resolved
@ogonkov ogonkov force-pushed the refactor/pin_vars branch from e033e64 to d578050 Compare May 21, 2024 06:55
Comment on lines +87 to +90
var(#{$-pin-mixin-variables-ns}-top-left)
var(#{$-pin-mixin-variables-ns}-top-right)
var(#{$-pin-mixin-variables-ns}-bottom-right)
var(#{$-pin-mixin-variables-ns}-bottom-left)
Copy link
Contributor

Choose a reason for hiding this comment

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

RTL support degradation

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Why?

Copy link
Contributor

Choose a reason for hiding this comment

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

physical properties instead of logical ones

Copy link
Contributor Author

Choose a reason for hiding this comment

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

But property is border-radius, it is rtl compatible

Copy link
Contributor

@amje amje Jun 3, 2024

Choose a reason for hiding this comment

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

There should be no such api like top-left, bottom-right. Instead we should only use logical names: start-start, end-end, etc.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh, i will rename vars, no problem

@each $selector in $selectors {
&#{get-pin-selector(#{$block}_pin_round-round, $selector, $append)} {
border-radius: $radius;
@include -pin-mixin-variables-from-list($radius);
Copy link
Contributor

Choose a reason for hiding this comment

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

This would break customization via CSS API, see "Custom" story of Button

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.

3 participants