-
-
Notifications
You must be signed in to change notification settings - Fork 4.8k
Allow controlling where responsive utilities are rendered using optional @tailwind screens; directive
#88
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
Changes from 5 commits
73f0ee9
471a19e
76eb4be
42a7038
6cec22b
44c0035
498c53c
ed71ee4
55f3f93
7e759ac
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,11 @@ | ||
| @responsive { | ||
| .example { | ||
| color: red; | ||
| } | ||
| } | ||
|
|
||
| @tailwind screen-utilities; | ||
|
|
||
| .john { | ||
| content: "wick"; | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,31 @@ | ||
| .example { | ||
| color: red; | ||
| } | ||
|
|
||
| @media (min-width: 576px) { | ||
| .sm\:example { | ||
| color: red; | ||
| } | ||
| } | ||
|
|
||
| @media (min-width: 768px) { | ||
| .md\:example { | ||
| color: red; | ||
| } | ||
| } | ||
|
|
||
| @media (min-width: 992px) { | ||
| .lg\:example { | ||
| color: red; | ||
| } | ||
| } | ||
|
|
||
| @media (min-width: 1200px) { | ||
| .xl\:example { | ||
| color: red; | ||
| } | ||
| } | ||
|
|
||
| .john { | ||
| content: "wick"; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -17,3 +17,15 @@ it('generates the right CSS', () => { | |
| expect(result.css).toBe(expected) | ||
| }) | ||
| }) | ||
|
|
||
| it('generates the right CSS with implicit screen utilities', () => { | ||
| const input = fs.readFileSync(path.resolve(`${__dirname}/fixtures/tailwind-input-with-explicit-screen-utilities.css`), 'utf8') | ||
|
|
||
| return postcss([tailwind()]) | ||
| .process(input) | ||
| .then(result => { | ||
| const expected = fs.readFileSync(path.resolve(`${__dirname}/fixtures/tailwind-output-with-explicit-screen-utilities.css`), 'utf8') | ||
|
|
||
| expect(result.css).toBe(expected) | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This test is not quite proving what you want; try adding some extra CSS to the end of the input fixture to make sure that the responsive utilities all show up before that extra CSS and I think you'll see what I mean! You'll need to update the output fixture too, or maybe duplicate it and add the new CSS to the end so we have separate fixtures for both scenarios. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. mmhhh, the problem here is that duplicating it would mean changing anything in two places in the future. Or am I wrong? I understood that the test does not properly test my feature. I'll hack around a bit, but duplicatiing is not a good option IMO ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. holy... this will get a mess. The problem is that the lib does not know if i used Do you have a hint how you'd tackle this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Okay, I'm wrong. |
||
| }) | ||
| }) | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,11 +7,12 @@ import buildMediaQuery from '../util/buildMediaQuery' | |
| export default function(config) { | ||
| return function (css) { | ||
| const screens = config().screens | ||
| const rules = [] | ||
| const responsiveRules = [] | ||
| let finalRules = [] | ||
|
|
||
| css.walkAtRules('responsive', atRule => { | ||
| const nodes = atRule.nodes | ||
| rules.push(...cloneNodes(nodes)) | ||
| responsiveRules.push(...cloneNodes(nodes)) | ||
| atRule.before(nodes) | ||
| atRule.remove() | ||
| }) | ||
|
|
@@ -23,13 +24,27 @@ export default function(config) { | |
| }) | ||
|
|
||
| mediaQuery.append( | ||
| rules.map(rule => { | ||
| responsiveRules.map(rule => { | ||
| const cloned = rule.clone() | ||
| cloned.selectors = _.map(rule.selectors, selector => `.${screen}\\:${selector.slice(1)}`) | ||
| return cloned | ||
| }) | ||
| ) | ||
| css.append(mediaQuery) | ||
|
|
||
|
|
||
| finalRules.push(mediaQuery) | ||
| }) | ||
|
|
||
| let includesScreenUtilitiesExplicitly = false | ||
| css.walkAtRules('tailwind', atRule => { | ||
| if (atRule.params === 'screen-utilities') { | ||
|
||
| includesScreenUtilitiesExplicitly = true | ||
| atRule.replaceWith(finalRules) | ||
| } | ||
| }) | ||
|
|
||
| if(! includesScreenUtilitiesExplicitly) { | ||
| css.append(finalRules) | ||
| } | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This test case is exactly what I was going to suggest! 😄 Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
:-D Testcase should be good now.
I'll make it green now ;-)
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@adamwathan Yeah, test is green now. Could you review again and please do not hesitate to correct me again or give me hints where to improve.