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

chore(css): Add grunttask to fix content-server Tailwind selectors #16466

Merged
merged 1 commit into from
Feb 22, 2024

Conversation

LZoog
Copy link
Contributor

@LZoog LZoog commented Feb 22, 2024

Because:

  • Our content-server CSS minification build task is stripping a space between brackets and asterisks, invalidating the selector and causing some styles to not apply. See comment included in commit for more details

This commit:

  • Adds a grunttask to add the whitespace back

fixes FXA-9167, fixes FXA-9170


In #16445, I wondered if removing --minify would not fix the problem (see the "EDIT" and comments). We included the fix in a dot release and I've since confirmed a few things:

  • Stage is still having the problems described in the linked tickets
  • --minify does not do anything for us for content-server at least, I compared what was uploaded to stage before and after this option and there is no diff because...
  • We're using a grunt task to minify our content-server CSS not handled by webpack
  • This bug was introduced because of the recent Tailwind upgrade, see this. Before this upgrade, our ltr and rtl classes did not produce :where([dir="ltr"], [dir="ltr"] *) selectors. It's possible this selector output (specifically the asterisk) is a bug in Tailwind but I'd need to do more investigating.

I tried every available formatting option for the cssmin task and also tried tweaking the level to prevent needing a new task, and nothing addressed this problem. I also tried excluding the Tailwind file from the cssmin task but couldn't get this to work.

I started adjusting the classes themselves to not use these selectors, but we've got a few that we'd have to change to something like [dir='rtl'] & {, and then we'd have to know and abide by not use these Tailwind classes in content-server due to the minify problem.

This seemed like a reasonable solution for now since we hope to sunset content-server soon anyway. Other options were described above, or downgrading Tailwind and pinning us to a version until we've sunsetted content-server or until/unless they tweak the selector.

This fix can be verified by running yarn build before and after this change in content-server and checking the dist/styles/[hash].tailwind.out.css file. See that before, there are several ]* references and after, they've become ] *.

@LZoog LZoog requested a review from a team as a code owner February 22, 2024 19:47
Because:
* Our content-server CSS minification build task is stripping a space between brackets and asterisks, invalidating the selector and causing some styles to not apply. See comment included in commit for more details

This commit:
* Adds a grunttask to add the whitespace back

fixes FXA-9167, fixes FXA-9170
Copy link
Contributor

@vpomerleau vpomerleau left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks for all your investigation on this, was not as straightforward as it first appeared.

I've compared the before/after files with diff checker (only difference is ]* --> ] * as expected).

@LZoog LZoog merged commit 638f7fb into main Feb 22, 2024
20 checks passed
@LZoog LZoog deleted the FXA-9170-v2 branch February 22, 2024 22:19
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.

2 participants