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(@clayui/css): Update and add box-shadow variable to cadmin class #5677

Closed
wants to merge 1 commit into from

Conversation

HenriquePereiraa
Copy link
Member

@@ -329,8 +329,9 @@ $cadmin-component-transition: color 0.15s ease-in-out,

$cadmin-component-focus-box-shadow: #{0 0 0 2px $cadmin-white,
0 0 0 4px $cadmin-primary-l0} !default;
$cadmin-component-focus-inset-box-shadow: #{inset 0 0 0 2px $cadmin-primary-l0,
inset 0 0 0 4px $cadmin-white} !default;
$cadmin-component-focus-inset-box-shadow: #{inset 0 0 0 0.125rem
Copy link
Member

Choose a reason for hiding this comment

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

We shouldn't change to rem value here. How it works is if you change the font-size on the html element, the value will scale based on it. In cadmin, we are trying to make it hard to overwrite admin styles via site styles. This is why we use px values.

@@ -80,6 +80,11 @@ $cadmin-clay-range-input: map-deep-merge(
position: absolute,
top: 50%,
width: 50%,
focus: (
Copy link
Member

Choose a reason for hiding this comment

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

You are right about something wonky going on with clay-range-variant or clay-range-input-variant. Something might be off with the way we are merging here

$focus: setter(map-deep-get($map, form-control-range, focus), ());
$focus: map-merge(
$focus,
(
outline:
setter(
map-get($map, focus-outline),
map-get($focus, outline)
),
)
);
$focus-clay-range-thumb: setter(
map-deep-get($map, focus, clay-range-thumb),
()
);
$focus-clay-range-thumb: map-merge(
$focus-clay-range-thumb,
(
box-shadow:
setter(
map-get($map, focus-thumb-box-shadow),
map-get($focus-clay-range-thumb, box-shadow)
),
)
);
.

form-control-range: (
appearance: none,
background-color: transparent,
height: if(variable-exists(input-height), $input-height, 2.375rem),
margin: 0,
padding: 0,
position: relative,
z-index: $zindex-clay-range-input-form-control,
hover: (
cursor: $link-cursor,
),
focus: (
outline: 0,
clay-range-thumb: (
box-shadow: $component-focus-box-shadow,
),
),
works but
focus: (
clay-range-thumb: (
box-shadow: $component-focus-inset-box-shadow,
),
doesn't.

It's weird and convoluted because we are keeping backward compatibility with Clay 2.x.

@@ -168,6 +162,16 @@ $cadmin-clay-range-input: map-deep-merge(
right: 0,
),
),
focus: (
Copy link
Member

Choose a reason for hiding this comment

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

@HenriquePereiraa This is working for me on cadmin. You probably need to restart the storybook / server to get the CSS to recompile.

Something that is curious to me here is that we seem to be circumventing a merge problem in the mixin by declaring it this way.

@HenriquePereiraa HenriquePereiraa marked this pull request as ready for review September 22, 2023 18:17
@pat270
Copy link
Member

pat270 commented Sep 22, 2023

Closing this one in favor of #5682

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.

@clayui/css: Cadmin focus-visible Clay Range doesn't show outline on tab
2 participants