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

Add default box shadow reset #81

Merged
merged 2 commits into from Sep 21, 2021
Merged

Add default box shadow reset #81

merged 2 commits into from Sep 21, 2021

Conversation

RobinMalfait
Copy link
Contributor

@RobinMalfait RobinMalfait commented Sep 16, 2021

This PR adds a reset for the shadow to each input element, this is necessary because we are trying to get rid of the universal selector.

See: tailwindlabs/tailwindcss#5517
Fixes: tailwindlabs/tailwindcss#5042

@vercel
Copy link

vercel bot commented Sep 16, 2021

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/tailwindlabs/tailwindcss-forms/ALZxhNrELXRTnsb4jfiKFgDmvev4
✅ Preview: https://tailwindcss-forms-git-add-box-shadow-reset-tailwindlabs.vercel.app

@RobinMalfait RobinMalfait changed the title add default box shadow reset Add default box shadow reset Sep 16, 2021
@adamwathan
Copy link
Member

We'll need to add this to the other elements that use shadows for focus rings too, like checkboxes/radios/maybe others 👍🏻

@RobinMalfait
Copy link
Contributor Author

@adamwathan Good catch! Updated the others as well, only added it for the "base" elements (so not for the ones with :checked or other pseudo's)

@RobinMalfait
Copy link
Contributor Author

RobinMalfait commented Sep 16, 2021

Before After
image image

Notice the incorrect shadow when the input has focus.

src/index.js Outdated
@@ -237,6 +240,7 @@ const forms = plugin.withOptions(function (options = { strategy: 'base' }) {
padding: '0',
'font-size': 'unset',
'line-height': 'inherit',
'--tw-shadow': '0 0 #0000',
Copy link
Member

Choose a reason for hiding this comment

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

Is this one needed? I can't think for sure but maybe it's not since we don't use box-shadow for any focus styles on file inputs?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Aha, yeah I added it because I wanted to make sure that I got all the base cases, but we are indeed not adding any focus styles for this one.
However if we eventually do, probably not though, then we can't forget to add these back.

@adamwathan
Copy link
Member

Looking good man thanks! I think we can simplify the box shadow calls to this now:

- 'box-shadow': `var(--tw-ring-offset-shadow), var(--tw-ring-shadow), var(--tw-shadow, 0 0 #0000)`,
+ 'box-shadow': `var(--tw-ring-offset-shadow), var(--tw-ring-shadow), var(--tw-shadow)`,

...since --tw-shadow will always be set. Super minor but code does nothing and makes the CSS bigger, so better to delete than have to wonder why it's there in the future when the details of this are out of mind.

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.

Shadow from modal inherited on inputs inside that modal
2 participants