Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Docs] Last move of tokens, Colors, to single page #5378

Merged
merged 16 commits into from
Nov 23, 2021

Conversation

cchaos
Copy link
Contributor

@cchaos cchaos commented Nov 15, 2021

Docs

Added Colors token page

Screen Shot 2021-11-15 at 16 44 52 PM

Also moved & simplified the "Contrast checker" here and hooked up to Language toggle

Screen Shot 2021-11-15 at 16 53 39 PM

Added Color mode specific page

For now it just contains the the example of swapping color mode at lower levels. Eventually this should contain instructions on how to setup the color switcher.

Changed “Global values” to “Customizing” to focus on just adjusting values

I'm still not completely happy with this page, I think it could be simpler, but I didn't want to bog down the token description pages with customization, and it also doesn't keep across pages. Maybe someday we could merge them.

  • HELP: Something has happened to the updater again where the calculated color pickers aren't updating downstream

Updated Color functions page to utilize the language switcher

In doing so, I also create a new doc tab type of SASS for easily passing in those snippets.

Screen Shot 2021-11-15 at 17 03 47 PM

Removed Sass page

All these parts were moved to either the token pages or "Customizing" page knowing that even more docs about setup and such are coming in #5121.

Move Sass vis tokens to Color palettes page

The only part that didn't really fit into tokens were the visualization token colors which have been moved to /utilities/color-palettes#sass-variables.

Rearranged a bunch of files and converted what was possible to TSX

A lot of files were touched because I spent some time re-organizing them a bit better and converting what I could to typescript.


Components

[EuiToolTip] Force nested EuiHorizontalRule backgrounds to be more subdued (match that of the title border)


Checklist

  • Check against all themes for compatibility in both light and dark modes
  • Checked in mobile
  • Checked in Chrome, Safari, Edge, and Firefox
  • Props have proper autodocs and playground toggles
  • Added documentation
  • Checked Code Sandbox works for any docs examples
  • [ ] Added or updated jest and cypress tests
  • [ ] Checked for breaking changes and labeled appropriately
  • [ ] Checked for accessibility including keyboard-only and screenreader modes
  • A changelog entry exists and is marked appropriately

@cchaos cchaos marked this pull request as ready for review November 15, 2021 22:10
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

@cee-chen
Copy link
Contributor

Re:

HELP: Need to hook these tabs into routing

Does it make sense to wait for @miukimiu's guidelines PR to merge in since you'll get a tabbed page API that auto-hooks into routing & search at that point?

@cchaos
Copy link
Contributor Author

cchaos commented Nov 18, 2021

Does it make sense to wait

Yep! I'll remove that item from the PR summary.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

Copy link
Contributor

@elizabetdev elizabetdev left a comment

Choose a reason for hiding this comment

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

Looked at "Color" and "Color functions" respective pages in Chrome, Safari, Edge, and, Firefox. Also looked at the code.

LGTM! 🎉

return {
id: color,
token: `colors.${color}`,
type: props[color],
Copy link
Contributor

Choose a reason for hiding this comment

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

The last two colors from this table are not getting the type:

Screenshot 2021-11-22 at 18 41 56

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They don't technically have a type because they're hard-coded to these values.

@thompsongl
Copy link
Contributor

HELP: Something has happened to the updater again where the calculated color pickers aren't updating downstream

Looking into this. Not much changed so I'm not quite sure what's going on yet

@cchaos
Copy link
Contributor Author

cchaos commented Nov 22, 2021

Not much changed so I'm not quite sure what's going on yet

Yeah, it's actually existing in master right now. The rendered hash color values as text updates, but the EuiColorPicker does not.
OjmyLIdOP_6ZypsR

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

Copy link
Contributor

@thompsongl thompsongl left a comment

Choose a reason for hiding this comment

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

LGTM!

@cchaos
Copy link
Contributor Author

cchaos commented Nov 22, 2021

Thanks @miukimiu ! And thanks @thompsongl for also fixing customize page. I think it's a little wonky that the color picker's popover auto closes after every change (it also makes it near impossible to use the keyboard), but maybe that's a limitation of the EuiColorPicker itself that would need to be fixed there?

@thompsongl
Copy link
Contributor

Oh I didn't notice that. But it is not a limitation of the color picker itself. I forced ThemeValue to rerender on updates (it wasn't previously) and it's resetting the popover open state. I'll take another look.

@thompsongl
Copy link
Contributor

9e6c79b resolves the color picker state issues

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

@cchaos
Copy link
Contributor Author

cchaos commented Nov 23, 2021

Works perfectly, thank you @thompsongl !

@cchaos cchaos enabled auto-merge (squash) November 23, 2021 17:29
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

…olors

# Conflicts:
#	CHANGELOG.md
#	src-docs/src/routes.js
@cchaos cchaos force-pushed the theme_languages/6_colors branch from 44a49b4 to 620ef83 Compare November 23, 2021 21:22
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_5378/

@cchaos cchaos merged commit 7b64d73 into elastic:main Nov 23, 2021
@cchaos cchaos deleted the theme_languages/6_colors branch November 23, 2021 23:01
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.

5 participants