feat: add support to assert styles on states (hover, active) and media queries #168

ankeetmaini · 2020-04-22T18:35:49Z

Fixes #138. Let me know if this approach looks okay.

itsdouges · 2020-04-22T20:54:41Z

hey mate thanks for throwing this PR up! I'll have a look

what's your thoughts on the API? I just threw something into the issue but I didn't really think about it.

itsdouges · 2020-04-22T21:00:54Z

packages/jest/src/matchers.tsx

-const containsClassNames = (classNames: string[], css: string) =>
-  classNames.reduce((accum, className) => (css.includes(`.${className}`) ? true : accum), false);
-
+// sorry but using ? was throwing TS off


the ? after MatchFilter?

itsdouges · 2020-04-22T21:04:02Z

packages/jest/src/matchers.tsx

+      let matcher = c;
+      if (state) matcher += `:${state}`;
+      if (media) matcher = `${media}{.*${matcher}`;
+      if (css.match(matcher)) {


this means if there is no matcher defined it will just mark all styles it finds as assertable yeah?

what's your thoughts on this? i can see two sides to it

it might give false positives so people think something is working when it isnt

we treat matchers as narrowing and its ok that the default case asserts on every state

Thanks for reviewing. I think a better approach would be to not do a text search and to convert this into an AST and then work with rules. This is done both in emotion-jest and styled-component jest matchers.

Also, at least in jest-styled-components matchers are mandatory and it doesn't work without narrowing, which I like too!

Let me know your thoughts and I can give it a spin.

i like it - let's give it a spin

i prefer explicit over implicit.

maybe we can also do some dev tooling help such as if we find what they're looking for in a state they haven't chosen suggest maybe looking there (can be done in another PR!)

ankeetmaini · 2020-04-23T16:20:19Z

@Madou this is ready for review now!

itsdouges · 2020-04-23T21:36:31Z

packages/jest/src/index.tsx

+      toHaveCompiledCss(
+        property: string,
+        value: string,
+        matchFilter?: Partial<Record<'state' | 'media', string>>


can bring in Pseudos to make it a bit easier and then fallback to string?

import { Pseudos } from 'csstype'; type State = Pseudos;

it does mean though every state will be prefix with a colon however : - maybe thats ok. (should we handle adding one if its not there?)

should we rename state to something that encompasses other groups pseudo selectors as well? what's your thoughts

Wow, state to 'selector' maybe? I'll try this and update

@Madou if we just use the value as Pseudos, TS will automatically suggest and add :, but then > :first-child this won't be a valid one.

Is there a way I can preserve the intellisense as well as adding string?

Pseudos | string is just string

let me check it out locally and look

found an issue microsoft/TypeScript#29729

import { Pseudos } from 'csstype'; /** * {} is used to enabled the compiler to not reduce Target down to a string. * This keeps both Pseduos intellisense working as well as then allowing us to define anything. * See: https://github.com/microsoft/TypeScript/issues/29729#issuecomment-460346421 */ type AnyTarget = string & { _?: never }; type Target = Pseudos | AnyTarget; export interface MatchFilter { target?: Target; media?: string; }

this fixes it

itsdouges · 2020-04-23T21:38:07Z

packages/jest/src/matchers.tsx

@@ -1,3 +1,10 @@
+import CSS from 'css';
+
+type MatchFilter = Partial<Record<'state' | 'media', string>>;


these types are duplicated in src/index.tsx maybe we can make a types.tsx file?

itsdouges · 2020-04-23T21:39:35Z

looks great man! really impressed. left a few comments 👍

as a side I need you to sign the contribution CLA - if you can have a read of https://github.com/atlassian-labs/compiled-css-in-js/blob/master/CONTRIBUTING.md and then sign "for individuals" will be great 👍

itsdouges · 2020-04-23T21:40:48Z

packages/jest/src/matchers.tsx

+  };
+
+  const allRules = getAllRules();
+  const klass = state ? `.${className}:${state}` : `.${className}`;


will we be able to assert or more complex selectors?

css={` > :first-child { color: red; } `} expect(element).toHaveCompiledCss('color', 'red', { state: '> :first-child' });

Does that work? If not should it? (maybe we need to rename state to selector.. or target.. or something..)

Wow, let me try this as well

Rename state -> target.

itsdouges · 2020-04-25T22:09:20Z

you've done a really great job!! ❤️

ankeetmaini · 2020-04-26T07:19:06Z

Yay, thanks so much @Madou :D

itsdouges · 2020-04-26T07:22:17Z

no worries! thank you for doing the work 🤗

ankeetmaini force-pushed the improve-matching branch from 964470f to e7ffa43 Compare April 22, 2020 18:37

itsdouges reviewed Apr 22, 2020

View reviewed changes

ankeetmaini force-pushed the improve-matching branch from 12b877d to e5282a3 Compare April 23, 2020 15:57

itsdouges reviewed Apr 23, 2020

View reviewed changes

ankeetmaini added 12 commits April 25, 2020 17:54

Remove double nesting for matching compiled css

4026f61

Add media and state for asserting styles.

0429af1

Fix TS error

08fd35c

Duplicate as TS is erroring

112cb1e

Add css module

ecdba05

Refactor matching logic to work on ast with rules.

16a2d79

Add test for state and media

2f91ce5

Move @types/css to dev dependencies

776f2c9

Improve tests a bit

aeb2113

Use Pseudo for awesome TS intellisense

d4d376a

Rename state -> target.

Add support for nested media querie assertions

8e9612f

Complex direct descendents etc work now.

3363921

ankeetmaini force-pushed the improve-matching branch from 7851c98 to 3363921 Compare April 25, 2020 12:28

Make intellisense work again, thanks @Madou

48f55f9

itsdouges approved these changes Apr 26, 2020

View reviewed changes

itsdouges merged commit b38c0d8 into atlassian-labs:master Apr 26, 2020

ankeetmaini deleted the improve-matching branch April 26, 2020 12:00

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add support to assert styles on states (hover, active) and media queries #168

feat: add support to assert styles on states (hover, active) and media queries #168

ankeetmaini commented Apr 22, 2020

itsdouges commented Apr 22, 2020

itsdouges Apr 22, 2020

ankeetmaini Apr 23, 2020

itsdouges Apr 22, 2020

ankeetmaini Apr 23, 2020

itsdouges Apr 23, 2020 •

edited

Loading

ankeetmaini commented Apr 23, 2020

itsdouges Apr 23, 2020 •

edited

Loading

ankeetmaini Apr 24, 2020

ankeetmaini Apr 25, 2020

itsdouges Apr 25, 2020

itsdouges Apr 25, 2020

itsdouges Apr 25, 2020 •

edited

Loading

itsdouges Apr 23, 2020 •

edited

Loading

itsdouges commented Apr 23, 2020 •

edited

Loading

itsdouges Apr 23, 2020 •

edited

Loading

ankeetmaini Apr 24, 2020

itsdouges commented Apr 25, 2020

ankeetmaini commented Apr 26, 2020

itsdouges commented Apr 26, 2020

		@@ -1,3 +1,10 @@
		import CSS from 'css';

		type MatchFilter = Partial<Record<'state' \| 'media', string>>;

feat: add support to assert styles on states (hover, active) and media queries #168

feat: add support to assert styles on states (hover, active) and media queries #168

Conversation

ankeetmaini commented Apr 22, 2020

itsdouges commented Apr 22, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsdouges Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

ankeetmaini commented Apr 23, 2020

itsdouges Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsdouges Apr 25, 2020 • edited Loading

Choose a reason for hiding this comment

itsdouges Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

itsdouges commented Apr 23, 2020 • edited Loading

itsdouges Apr 23, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

itsdouges commented Apr 25, 2020

ankeetmaini commented Apr 26, 2020

itsdouges commented Apr 26, 2020

itsdouges Apr 23, 2020 •

edited

Loading

itsdouges Apr 23, 2020 •

edited

Loading

itsdouges Apr 25, 2020 •

edited

Loading

itsdouges Apr 23, 2020 •

edited

Loading

itsdouges commented Apr 23, 2020 •

edited

Loading

itsdouges Apr 23, 2020 •

edited

Loading