elastic / eui

Elastic UI Framework 🙌
https://eui.elastic.co/
Other
6.09k stars 837 forks source link

Exporting some utility helpers #6837

Open dej611 opened 1 year ago

dej611 commented 1 year ago

Is your feature request related to a problem? Please describe.

While on terms of usability and accessibility the framework is improving, it is becoming challenging to test on these aspects as they are now computing at runtime. For instance button colours are now computed at runtime with chroma and emotion to improve the contrast ratio: https://github.com/elastic/eui/blob/main/src/themes/amsterdam/global_styling/mixins/button.ts#L76

For instance in this Kibana test I had to give up with a static value check, like a snapshot test, as computing the actual value would just duplicate a logic here in EUI, which would be even worst: https://github.com/elastic/kibana/blob/66ae25d376ecc1ea8ca82d60e7fb4edfa09192d3/x-pack/plugins/lens/public/shared_components/dataview_picker/trigger.test.tsx#LL61C67-L61C83

But some can be applied to other components like the select component with multiple selections. Etc..

Describe the solution you'd like It would be nice to have some helpers to simplify the testing on the consumer side. For colors that could be publishing all computed variants in a euiThemeDebugVars json, or just provide a function helper. For other component some other solution might help.

I think that some dog fooding here would help, as you are probably the best to find the right helper API to test the component on your side. Just exporting them would already make things much better.

cee-chen commented 1 year ago

In general, I'm 100% on board with the premise of exporting more test utility helpers, especially if it would help consumers move away from snapshots, and it feels like something we could absolutely do alongside our general Enzyme->RTL tech debt work.

Just to clarify the specific use case you linked/mentioned in your PR description - it's not clear to me what the goal is with the color checking assertions 🤔 TBH, with most things CSS-based I would have leaned towards a visual screenshot/diff rather than trying to check the specific CSS output.

In your opinion, would it be fine for an exported consumer assertion to check the rendered Emotion className (e.g. a danger class) rather than the actual rendered CSS, and have EUI's internal unit tests be the ones in charge of confirming that the rendered CSS outputs the expected color?

dej611 commented 1 year ago

TBH, with most things CSS-based I would have leaned towards a visual screenshot/diff rather than trying to check the specific CSS output.

Not sure we have an infrastructure for that in Kibana testing.

In your opinion, would it be fine for an exported consumer assertion to check the rendered Emotion className (e.g. a danger class) rather than the actual rendered CSS, and have EUI's internal unit tests be the ones in charge of confirming that the rendered CSS outputs the expected color?

No preference here as the only implementation detail I would like to know when testing is that a danger styling has been applied to the text. If that is passing thru a style or class check would be the same for me.

github-actions[bot] commented 10 months ago

👋 Hi there - this issue hasn't had any activity in 6 months. If the EUI team has not explicitly expressed that this is something on our roadmap, it's unlikely that we'll pick this issue up. We would sincerely appreciate a PR/community contribution if this is something that matters to you! If not, and there is no further activity on this issue for another 6 months (i.e. it's stale for over a year), the issue will be auto-closed.

JasonStoltz commented 10 months ago

@tkajtoch FYI, we've been talking about test helpers.