Skip to content

Conversation

@cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Dec 5, 2024

Summary

Note

This PR merges into a feature branch.

@mgadewoll made the excellent suggestion last PR to add some utilities that reduce the number of high contrast mode ternaries in our styles. 🎉

While technically these "DRY" utils add more net lines, these are mostly due to formatting/syntax. Overall, I find this significantly more pleasant to read and easier to understand than the previous ternaries, although of course this is subjective - I'm open to feedback on this.

I'm particularly also looking for feedback on the names, as that's the hardest part of coding 😅

QA

Unit tests and VRT should have caught any issues, but if you want to quickly smoke test some touched components for
any regressions:

General checklist

  • Browser QA - N/A, see above QA section
  • Docs site QA - N/A
  • Code quality checklist
  • Release checklist - N/A, since these will not be public EUI exports
  • Designer checklist - N/A

- not totally sure why VRT is registering new changes - it looks the same to me?

- no changes to EuiSwitch 🤷‍♂️
- some fun shenanignas with our `euiButton` color utils returning objects instead of strings, and Emotion _really_ not liking that / requiring manual serialization to a string

- also not sure why VRT is being finicky here, but might as well update everything. SuperDatePicker hasn't been updated in a while I think
@cee-chen cee-chen marked this pull request as ready for review December 5, 2024 16:16
@cee-chen cee-chen requested a review from a team as a code owner December 5, 2024 16:16
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks like a tiny difference in width somehow. I'd assume this is not an actual change but some rendering difference 🤷‍♀️

Screen.Recording.2024-12-05.at.20.34.51.mov

Copy link
Contributor

@mgadewoll mgadewoll left a comment

Choose a reason for hiding this comment

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

🚢 🐈‍⬛ I really like the utils! Thanks for iterating on the names and syntax, it definitely fits nicely now, imho. 🎉

@cee-chen cee-chen merged commit c4ffc12 into elastic:high-contrast-mode Dec 6, 2024
@cee-chen cee-chen deleted the high-contrast-utils branch December 6, 2024 16:51
@kibanamachine
Copy link

Preview staging links for this PR:

@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

cee-chen added a commit that referenced this pull request Dec 6, 2024
cee-chen added a commit that referenced this pull request Dec 12, 2024
@cee-chen cee-chen mentioned this pull request Dec 12, 2024
10 tasks
mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request Mar 14, 2025
mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request Mar 18, 2025
mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request Mar 18, 2025
mgadewoll pushed a commit to mgadewoll/eui that referenced this pull request Mar 20, 2025
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.

4 participants