Skip to content

Conversation

@cee-chen
Copy link
Contributor

@cee-chen cee-chen commented Sep 19, 2024

Summary

Code diffs can be a little hard to follow so I recommend following the QA steps below instead. I'm particularly looking for feedback on the developer experience and whether the copy/instructions would make sense to you as someone new to EUI (but not necessarily new to frontend dev).

QA

  • Go to https://eui.elastic.co/pr_8029/#/guidelines/getting-started
  • Confirm the Setting up your application section no longer has any imported .css files
  • Read through the Styling your application section and confirm that it makes sense with your understanding of EUI's usage and does not reference Sass.
  • Also confirm that all links work properly/as expected in the two above sections
  • Go to https://eui.elastic.co/pr_8029/#/theming/colors/values?themeLanguage=sass
  • Confirm that a red danger callout appears at the top warning devs away from using our Sass utilities/values
  • Switch the pink tab from Sass to CSS-in-JS and confirm the callout disappears
  • Other docs/copy QA
  • Spin up the website/EUI+ docs and confirm that the Getting Started page there matches src-doc's
  • Confirm the repo README no longer has a reference to the status of EUI's theme

General checklist

N/A, docs only

…migration

+ remove `colorMode` docs in anticipation of automatic system color mode logic, and link to EuiProvider props instead
+ move bottom bullet point to a location where it'll actually be read, and wrap it in a warning callout
- should be under styling section, where it's actually useful

- add link to cache config, since that affects CSS specificity

- modify example to be usable in codesandbox
+ refactor usage elsewhere
@cee-chen cee-chen added documentation Issues or PRs that only affect documentation - will not need changelog entries skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation) emotion labels Sep 19, 2024
@cee-chen cee-chen force-pushed the theming-docs-updates branch from 9de7e7d to 50bed56 Compare September 19, 2024 02:05
- no need to import `.css` files any longer

- remove context, use `useEuiTheme()` instead
+ remove duplicated README in `packges/eui` - half the links don't work anyway
@cee-chen cee-chen force-pushed the theming-docs-updates branch from 50bed56 to 6bbfad7 Compare September 19, 2024 02:10
@cee-chen cee-chen marked this pull request as ready for review September 19, 2024 02:36
@cee-chen cee-chen requested a review from a team as a code owner September 19, 2024 02:36
Copy link
Member

@JasonStoltz JasonStoltz left a comment

Choose a reason for hiding this comment

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

Hey Cee. These updates look comprehensive and I think we covered all of our bases here. Thanks for handling this!

@cee-chen cee-chen enabled auto-merge (squash) September 20, 2024 20:37
@cee-chen cee-chen force-pushed the theming-docs-updates branch from b82d871 to 21c7470 Compare September 20, 2024 20:39
@cee-chen cee-chen merged commit e6ccac8 into elastic:main Sep 20, 2024
@cee-chen cee-chen deleted the theming-docs-updates branch September 20, 2024 21:06
@elasticmachine
Copy link
Collaborator

💚 Build Succeeded

History

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Issues or PRs that only affect documentation - will not need changelog entries emotion skip-changelog Use on PRs to skip changelog requirement (Don't delete - used for automation)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants