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

Change color fn used to calculate icon colors for search typeahead #4884

Merged

Conversation

joshuarrrr
Copy link
Member

Description

Icons don't need to be as high contrast as text, so the 3:1 threshold of makeGraphicContrastColor is more appropriate

Issues Resolved

fixes #4628

Screenshot

These styles aren't actually currently used, because we don't have full typeahead support available in the query bar, except for historical queries. But I've manually updated some of the suggestions with the 4 color style classes affected by this change. The colors themselves haven't changed, but it fixes the error. Note that this change is only for the color variants - the contrast for the grayscale ones are probably worse, but defined elsewhere:

Screen Shot 2023-08-31 at 2 27 34 PM Screen Shot 2023-08-31 at 2 26 50 PM Screen Shot 2023-08-31 at 2 25 42 PM Screen Shot 2023-08-31 at 2 24 05 PM

Testing the changes

Before:

np bld    log   [21:29:23.956] [info][@osd/optimizer] starting worker [1 bundle]
np bld    log   [21:29:27.418] [warning][@osd/optimizer] worker stderr WARNING: High enough contrast could not be determined. Most likely your background color does not adjust for dark mode.
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr          on line 128:7 of node_modules/@elastic/eui/src/themes/eui-next/global_styling/functions/_colors.scss, in function `makeHighContrastColor`
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr          from line 105:16 of src/plugins/data/public/ui/typeahead/_suggestion.scss
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr          from line 1:9 of src/plugins/data/public/ui/typeahead/_index.scss
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr          from line 2:9 of src/plugins/data/public/ui/_index.scss
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr          from line 2:9 of src/plugins/data/public/index.scss
np bld    log   [21:29:27.419] [warning][@osd/optimizer] worker stderr 
np bld    log   [21:29:28.509] [warning][@osd/optimizer] worker stderr WARNING: High enough contrast could not be determined. Most likely your background color does not adjust for dark mode.
np bld    log   [21:29:28.509] [warning][@osd/optimizer] worker stderr          on line 128:7 of node_modules/@elastic/eui/src/themes/eui-next/global_styling/functions/_colors.scss, in function `makeHighContrastColor`
np bld    log   [21:29:28.509] [warning][@osd/optimizer] worker stderr          from line 105:16 of src/plugins/data/public/ui/typeahead/_suggestion.scss
np bld    log   [21:29:28.509] [warning][@osd/optimizer] worker stderr          from line 1:9 of src/plugins/data/public/ui/typeahead/_index.scss
np bld    log   [21:29:28.509] [warning][@osd/optimizer] worker stderr          from line 2:9 of src/plugins/data/public/ui/_index.scss
np bld    log   [21:29:28.510] [warning][@osd/optimizer] worker stderr          from line 2:9 of src/plugins/data/public/index.scss
np bld    log   [21:29:28.510] [warning][@osd/optimizer] worker stderr 
np bld    log   [21:29:32.591] [success][@osd/optimizer] 1 bundles compiled successfully after 10.2 sec, watching for changes

After:

np bld    log   [21:29:43.878] [success][@osd/optimizer] 1 bundles compiled successfully after 5.2 sec, watching for changes

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
    • yarn test:ftr
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

…ggestions

Icons don't need to be as high contrast as text, so the 3:1 threshold of `makeGraphicContrastColor` is more appropriate

Signed-off-by: Josh Romero <[email protected]>
@joshuarrrr
Copy link
Member Author

This doesn't need a changelog entry - it's really just getting rid of a sass bundling warning.

@codecov
Copy link

codecov bot commented Aug 31, 2023

Codecov Report

Merging #4884 (84b9709) into main (ce2d79e) will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #4884      +/-   ##
==========================================
- Coverage   66.39%   66.39%   -0.01%     
==========================================
  Files        3396     3396              
  Lines       64801    64801              
  Branches    10359    10359              
==========================================
- Hits        43025    43023       -2     
- Misses      19217    19218       +1     
- Partials     2559     2560       +1     
Flag Coverage Δ
Linux_1 34.85% <ø> (ø)
Linux_2 55.15% <ø> (ø)
Linux_3 44.23% <ø> (-0.01%) ⬇️
Linux_4 34.92% <ø> (ø)
Windows_1 34.87% <ø> (ø)
Windows_2 55.12% <ø> (ø)
Windows_3 44.23% <ø> (ø)
Windows_4 34.92% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

@ashwin-pc ashwin-pc merged commit b24f33a into opensearch-project:main Aug 31, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 31, 2023
…ggestions (#4884)

Icons don't need to be as high contrast as text, so the 3:1 threshold of `makeGraphicContrastColor` is more appropriate

Signed-off-by: Josh Romero <[email protected]>
(cherry picked from commit b24f33a)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
AMoo-Miki pushed a commit that referenced this pull request Sep 1, 2023
…ggestions (#4884) (#4886)

Icons don't need to be as high contrast as text, so the 3:1 threshold of `makeGraphicContrastColor` is more appropriate


(cherry picked from commit b24f33a)

Signed-off-by: Josh Romero <[email protected]>
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport 2.x distinguished-contributor Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry v2.10.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] makeHighContrastColor warns that it cannot make good colors for typeahead/_suggestion.scss
4 participants