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

Look & Feel - Sass variable value changes for research #807

Closed
Tracked by #4456
KrooshalUX opened this issue Jun 12, 2023 · 16 comments
Closed
Tracked by #4456

Look & Feel - Sass variable value changes for research #807

KrooshalUX opened this issue Jun 12, 2023 · 16 comments
Assignees

Comments

@KrooshalUX
Copy link
Contributor

KrooshalUX commented Jun 12, 2023

In the interest of delivering an impactful but quick change to look & feel of the OpenSearch Dashboards product, the UX team has determined an update to existing Sass variable values.

Proposed changes visualized:
Screen Shot 2023-06-12 at 4 43 46 PM

UI COLORS

  • Knowing that there are various aliased uses of these base colors, and that shading/tinting may occur these colors for the various uses (ex: ouiColorWarning is shaded drastically for applying to text) are subject to updates based on initial results and will be reflected within this issue as needed
  • There are some unknown visual impacts on components - for example the focus colors for things like Accordion, which on click the color of the icon is transformed from the text color to the primary color
  • Included in these core definitions, some specific color definitions for some aliases in order to address potential contrast issues using the current shade/tint approach in OUI - as well as separating the color use of Primary from Link
  • Do not use absolute white #FFF or absolute black #000 for any elements within the styling update. Please update UX team via comments if those values remain within the code after these updates and we will work on their appropriate replacement.

LIGHT MODE
ouiColorEmptyShade #FCFEFF
ouiColorLIghtestShade #E3E5E8
ouiColorLightShade #D6D9DD
ouiColorMediumShade #ADB4BA
ouiColorDarkShade #5A6875
ouiColorDarkestShade #2A3947
ouiColorFullShade #0A1219
ouiColorPrimary #159D8D
ouiColorSecondary #017D73
ouiColorAccent #DD0A73
ouiColorWarning #F5A700
ouiColorDanger #BD271E
ouiLinkColor #006BB4
ouiPageBackgroundColor #F0F2F4
ouiColorInk #0A121A
ouiColorGhost #FCFEFF

DARK MODE
ouiColorEmptyShade #0A121A
ouiColorLIghtestShade #101B25
ouiColorLightShade #293847
ouiColorMediumShade #5B6875
ouiColorDarkShade #8D98A3
ouiColorDarkestShade #DFE3E8
ouiColorFullShade #FCFEFF
ouiColorPrimary #159D8D
ouiColorSecondary #7DE2D1
ouiColorAccent #F990C0
ouiColorWarning #FFCE7A
ouiColorDanger #FF6666
ouiLinkColor #1BA9F5
ouiPageBackgroundColor #172430
ouiColorInk #0A121A
ouiColorGhost #FCFEFF

VIS COLORS
Tracked in another issue #818
Utilize OUI color palettes for iterative improvements https://oui.opensearch.org/1.2/#/utilities/color-palettes

  • related task - point OSD to use OUI visualization palettes for visual & accessibility testing

TYPOGRAPHY
Font sizing may need to be adjusted based on initial results.
ouiFont - Source Sans 3
ouiCodeFont - Source Code Pro

AMoo-Miki added a commit to AMoo-Miki/oui that referenced this issue Jun 13, 2023
opensearch-project#807

Signed-off-by: Miki <miki@amazon.com>
AMoo-Miki added a commit to AMoo-Miki/oui that referenced this issue Jun 13, 2023
opensearch-project#807

Signed-off-by: Miki <miki@amazon.com>
@BSFishy
Copy link
Contributor

BSFishy commented Jun 13, 2023

According to the code, ouiColorInk and ouiColorGhost must stay at #000 and #FFF. If I'm not mistaken, the reason is that these colors are used when shading or tinting other colors. I.e. they're used in the functions to make other colors lighter or darker. I'm thinking changing these might have unexpected impacts on colors but haven't tested

Additionally, if needed, we can add new variables in places where those variables are used directly: https://github.com/search?q=repo%3Aopensearch-project%2Foui+ouiColorInk+OR+repo%3Aopensearch-project%2Foui+ouiColorGhost&type=code

AMoo-Miki added a commit to AMoo-Miki/oui that referenced this issue Jun 13, 2023
opensearch-project#807

Signed-off-by: Miki <miki@amazon.com>
@KrooshalUX
Copy link
Contributor Author

@BSFishy Our contrast ratios for the new color scheme are not based on absolute black or white - we are trying to move away from those values for text especially. So, keeping those black and white may provide the wrong automatic adjustments to the colors provided. So, I would like us to try to implement this change and see what happens, we can adjust if the shades and tints end up funky.

@KrooshalUX
Copy link
Contributor Author

KrooshalUX commented Jun 14, 2023

Based on initial screenshots, I think this is going the right direction!
Notes

  • Breadcrumb is not yet picking up styling, there is an implementation change that needs to be pushed for this to be accurate. @AMoo-Miki is working on that I believe.
  • spoke with @BSFishy - Main has some slight differences in UI from playground.opensearch.org - including some spacing issues in Dashboard and a filled Refresh button (in playground its correctly a hollow button). We might want to branch from the 2.x line so comparisons against current Dashboards can be made more accurately.

Screen Shot 2023-06-13 at 5 31 37 PM
Screen Shot 2023-06-13 at 5 31 46 PM
Screen Shot 2023-06-13 at 5 33 16 PM
Screen Shot 2023-06-13 at 5 33 27 PM

@BSFishy curious to know if any of the supplied values were transformed after the compiling? It would be good for us to check those before moving forward on further changes.

@kgcreative I am thinking that ouiLInkColor should actually just point to Primary afterall, as the mix of Button and Link colors is kind of frustrating on my eye - also, since we have little control over how/when builders will use links vs button for navigation, this reduces visual impact. I think we might need to adjust our Primary slightly - or consider Semi Bold or Bold button text with the new font - but I would like to make that judgement once we have an endpoint vs screengrabs that may be altering colors during compression.

@BSFishy
Copy link
Contributor

BSFishy commented Jun 14, 2023

@BSFishy curious to know if any of the supplied values were transformed after the compiling? It would be good for us to check those before moving forward on further changes.

Diff of light theme: https://gist.github.com/BSFishy/484f02ee390ca1423cc9d888950f7865
Diff of dark theme: https://gist.github.com/BSFishy/2a3c81b510045abce24f2d9413388ab5

(updated gist links to remove Eui aliases)

@KrooshalUX
Copy link
Contributor Author

Round 2 style change: Remove my original suggestion to use a specific ouiLinkColor and allow it to point to Primary as the default.

@AMoo-Miki
Copy link
Collaborator

FYI, to normalize the size of Source Sans 3, I have given it a 110% size adjustment.

AMoo-Miki added a commit to AMoo-Miki/oui that referenced this issue Jun 15, 2023
opensearch-project#807

Signed-off-by: Miki <miki@amazon.com>
@AMoo-Miki
Copy link
Collaborator

Parts of OUI that don't honor the fonts set in SASS vars:

  • OuiText > blockquote
  • Anything using Ace or Monaco

@KrooshalUX
Copy link
Contributor Author

KrooshalUX commented Jun 15, 2023

Latest release feedback @AMoo-Miki @BSFishy @joshuarrrr :

  • Breadcrumb isn't using ouiColorPimary - looking at https://oui.opensearch.org/1.2/#/navigation/breadcrumbs it should be using a tint/shade based on ouiColorPrimary unless I am missing a specific definition of breadcrumb colors.
  • The font looks a bit blurry in breadcrumb - this issue has also occurred in the past - however the ouiColorGhost may be contributing to this. With some guidance I can recommend some tweaks.
    Breadcrumb using Source Sans 3: Screen Shot 2023-06-15 at 10 49 22 AM
    Breadcrumb using Inter: Screen Shot 2023-06-15 at 10 49 28 AM
  • OuiSuperUpdateButton text is perhaps being overridden still - the font color on the button is also different than the button color itself, and the button should be filled.
  • Nav should be updated to be single nav vs stacked nav
  • Change ouiText letter spacing to 0, currently it is set at -.005em
  • I would like to try a version where ouiButton font weight is bumped up to semi-bold - If this is a problematic customization or change to OUI, please let me know.
  • all ouiText should use Source Sans 3 - including blockquote
  • Anything using Ace or Monaco should use the ouiCodeFont if possible

Screen Shot 2023-06-15 at 10 32 59 AM

@AMoo-Miki
Copy link
Collaborator

Breadcrumb isn't using ouiColorPimary

The font looks a bit blurry in breadcrumb

Breadcrumbs are ouiLinks and use color prop of text or subdued. text uses $ouiColorGhost and subdued uses $ouiTextSubduedColor. Would you want us to override the text only inside breadcrumbs to $ouiColorPimary?

On a side note: a crumb with :focus shows am odd colored background that is not shaped as the crumb.

@AMoo-Miki
Copy link
Collaborator

OuiSuperUpdateButton text is perhaps being overridden still

Yes there is some rouge styling in a plugin: opensearch-project/anomaly-detection-dashboards-plugin#509

@AMoo-Miki
Copy link
Collaborator

Change ouiText letter spacing to 0, currently it is set at -.005em

The letter-spacing is applied by a mixin named ouiFont and has a wide reach. Would you prefer:

  1. I simply set it to 0 for ouiText, or
  2. remove the -.005em altogether?

@KrooshalUX
Copy link
Contributor Author

KrooshalUX commented Jun 15, 2023

Change ouiText letter spacing to 0, currently it is set at -.005em

The letter-spacing is applied by a mixin named ouiFont and has a wide reach. Would you prefer:

1. I simply set it to `0` for `ouiText`, or

2. remove the `-.005em` altogether?

@AMoo-Miki whatever makes the most sense for engineering ergonomics / code cleanliness from your perspective - with the caveat that if removing it all together impacts OuiTitle, then we should just set it to 0 specifically for OuiText.

@AMoo-Miki
Copy link
Collaborator

I would like to try a version where ouiButton font weight is bumped up to semi-bold

The weight now is 400. I have changed this to 600 which is what SemiBold is mapped to. FYI, we have a Medium too which is 500.

@AMoo-Miki
Copy link
Collaborator

if removing it all together impacts OuiTitle, then we should just set it to 0 specifically for OuiText.

OuiTitle had its own letter-spacing except for xxs and xxxs. I dropped letter-spacing from ouiFont and added the same to xxs and xxxs of OuiTitle.

@AMoo-Miki
Copy link
Collaborator

@KrooshalUX, .osdWelcomeView sets the background of the page when the OpenSearch logo spinner is shown. It has a hardcoded background color which differs from those of OSD itself.

background-color: ${darkMode ? '#1D1E24' : '#FFF'};

Would you rather we use the default background color on this?

@KrooshalUX
Copy link
Contributor Author

The first pass of color implementation has gone live, so closing this as completed. We will be tracking follow up tasks for improvements in a new issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Done
Development

No branches or pull requests

4 participants