chromaui / storybook-addon-pseudo-states

CSS pseudo-classes for Storybook
MIT License
90 stars 30 forks source link

Selecting specific element also applies pseudostate to all its descendants #56

Closed filipw01 closed 1 year ago

filipw01 commented 1 year ago

Because of this rewrite logic if .pseudo-* class is applied to a specific element it triggers all descendants' pseudostate https://github.com/chromaui/storybook-addon-pseudo-states/blob/main/src/rewriteStyleSheet.test.js#L26-L30

when: parameters = { pseudo: { hover: '.someSelector' } } or parameters = { pseudo: { hover: ['.someSelector', 'div'] } }

Current state: a:hover -> a:hover, a.pseudo-hover, .pseudo-hover a

Proposed change: a:hover -> a:hover, a.pseudo-hover

when: parameters = { pseudo: { hover: true } }

no changes - a:hover -> a:hover, a.pseudo-hover, .pseudo-hover a

filipw01 commented 1 year ago

I can work on this, just want to confirm that this is a valid option that you want to implement 😄

filipw01 commented 1 year ago

@ghengeveld What do you think about it?

poalrom commented 1 year ago

Hi! We faced the same issue with pseudostates and snapshotting with Chromatic. Play functions are not working for us (esp with hover) and this addon is the only solution. @ghengeveld, can you take a look, please?

ghengeveld commented 1 year ago

Hi @filipw01, sorry I missed this earlier. The problem you're describing and the solution you're suggesting both seems valid and appropriate. If you're still willing and able, feel free to take a stab at implementing it. Otherwise I can do this eventually, but I'm going to be on holiday soon. Maybe @poalrom if you're interested, you can work on it?

poalrom commented 1 year ago

@ghengeveld I can add this functionality, but I think it will break backward compatibility. Is it ok to up to a major version or it's better to add a different way to enable this behaviour?

JonathanKolnik commented 1 year ago

Hi everyone,

I'm Jono, one of Gert's colleagues on the Chromatic team. He is out this week, but I think we should move forward with this approach and up to a major version.

@poalrom thank you for offering to add the functionality, do you still have the bandwidth to do so? No worries if not, just let me know 😄

JonathanKolnik commented 1 year ago

I'm starting this today

JonathanKolnik commented 1 year ago

You can use the canary version here ~https://www.npmjs.com/package/storybook-addon-pseudo-states/v/2.1.1--canary.85.01f67b4.0, I am still working on support for custom elements but it should work otherwise.~

https://www.npmjs.com/package/storybook-addon-pseudo-states/v/2.1.1--canary.85.0bc3584.0 with custom elements working now.

Please let me know if you have any feedback #85

thafryer commented 1 year ago

^^ @poalrom @filipw01

poalrom commented 1 year ago

Thank you, it works like a charm!

JonathanKolnik commented 1 year ago

I'm closing this issue because it has been fixed and is now available in v2.1.1