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: update flash to play nicely with tailwind JIT purge #21

Merged
merged 1 commit into from
Jan 10, 2022

Conversation

superhawk610
Copy link
Contributor

In order for Tailwind JIT to include classnames, they must be included in full somewhere in a source file included on the purge list. This means that string interpolation won't be included (see https://v2.tailwindcss.com/docs/just-in-time-mode, heading titled "Dynamic values").

This PR also updates the flash helper to use the error icon for error flash messages, in place of a red check mark.

image

In order for Tailwind JIT to include classnames, they must be included
_in full_ somewhere in a source file included on the `purge` list. This
means that string interpolation won't be included (see
https://v2.tailwindcss.com/docs/just-in-time-mode, heading titled
"Dynamic values").
@superhawk610 superhawk610 added the bug Something isn't working label Jan 10, 2022
@nickolaich
Copy link
Contributor

nickolaich commented Jan 10, 2022

I dare to suggest a change in approach and use css features:
instead of bg_class/button_class/text_class, add a root class to flash errors:
flash-error
flash-info
etc

and implement it at app.scss

.flash-error {
  @apply bg-red-50;
}
.flash-error button{
  @apply text-red-1 focus:ring-offset-red-50 focus:ring-red-1;
}

We can easily add later any type of flashes: success/warning etc.

and probably Tailwind JIT will catch them automatically.

@superhawk610
Copy link
Contributor Author

@nickolaich that's a good idea, but crunch_berry doesn't ship any CSS files, so any styling has to be done inline. We could add those CSS rules into ecom_api, but then crunch_berry isn't self-contained. I'll start a discussion in the #crunch-berry-components channel in Slack to explore this idea further, though.

@superhawk610 superhawk610 merged commit 881ca3a into main Jan 10, 2022
@superhawk610 superhawk610 deleted the fix/flash-message-tailwind-jit-purge branch January 10, 2022 19:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants