-
Notifications
You must be signed in to change notification settings - Fork 159
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
(OUI Next Theme) Security #1527
Comments
Thanks for filing |
[Triage] The action items for this task are 1) to modify the SVG to be viewable on the dark background 2) swap auditlogs to updated item type. |
Hi @KrooshalUX, I am looking at the first request of the issue and ran into a question. I am not sure how to modify the svg as you requested... Looking at the settings, I see
I am not sure what you mean by modifying the color to |
Could |
I also see a reference for
|
@scrawfor99 - apologies, on second thought - 'euiColorGhost' would render it white in light and dark mode, which would not work for light mode. Let's try 'euiColorMediumShade' or 'euiColorDarkShade' and see how it comes out? https://oui.opensearch.org/1.2/#/guidelines/colors We may need to do some trial and error to see how it ends up appearing. |
Hi @davidlago, thanks for your suggestions. I had actually given those a shot and saw it referenced in the styleGuide, but I still don't know what the expected outcome is. For instance, I can swap everything to use the
But I am 99% sure that is not the desired outcome here. We probably want just some parts to be ghost but I am not sure which I should set for the new theme. Does that make sense? |
@scrawfor99 - @kgcreative may have additional change suggestions for the SVG, so tagging him in as well. |
Hi @KrooshalUX sounds good. I assume we will want something like this:
|
@scrawfor99 you probably only want to update the ones where I see in the styleguide that that color is I hope we are not having components figure out whether they are in dark mode or not to switch colors.. so hopefully |
Hi @davidlago, yeah I looked at swapping but the lines are also dark so I don't know if we want to swap those too? This is what the Ghost version would look like I assume? |
Yeah, I'd assume the lines and text both need to be changed. That looks good to me... how does it look for light mode? |
@davidlago @scrawfor99 using 'euiColorDarkShade' or 'euiColorMediumShade' does adjust automatically for light and dark modes. My earlier (now updated) instruction to use 'euiColorGhost' was incorrect - as it would be near-white in both light and dark mode ('euiColorGhost' and 'euiColorInk' do not adjust for light/dark mode) You can see a preview of the adjustment by using the theme switcher within the OUI documentation https://oui.opensearch.org/1.2/#/guidelines/colors |
Thanks @KrooshalUX @davidlago & @scrawfor99 -- here's an updated SVG -- I've changed all the fills and strokes to "currentColor" -- which should simply inherit the values you set in CSS. I've also removed the "blue" circles and made them the current color as well so we don't need to worry about matching colors if the theme changes in the future. Please let me know if this works (The github preview probably won't render correctly, but the .svg should work. I'd love to see some screenshots once you've tried it in local.
|
Hi @kgcreative, unfortunately the swap just makes it not show up at all. I went and made the changes manually to the existing SVG but also don't know that it is what we will want: I would think we would want "oppositeColor" if such a thing existed? |
@scrawfor99 Thanks for posting the screenshots - to confirm, is this approach using sass variables from OUI The OUI Sass variables provided have different hex values based on light or dark mode applied, so they already are self-adjusting. These will also adjust depending on theme, which we have a new one launching soon - and any sort of customized color may not provide the correct contrast. Examples from https://oui.opensearch.org/1.2/#/guidelines/colors : @kgcreative or @AMoo-Miki - can you provide further guidance? |
My latest .svg removes the blue-color circles, yes, but the caveat here is you may need to inline it in order to actually correctly show the functionality. You can't do it as an the nice thing about this, is that if you set color="cssVariableName", then when that variable changes in the theme, you'll get that for free. @AMoo-Miki - do you know if we have any utilities in OSD to inline SVGs? |
Hi @KrooshalUX, @AMoo-Miki, and @kgcreative, I just took the hex from the theme and did it manually. I recognize this may not be what you intended however... For you suggestion Kevin, I am not sure how to inline the SVG since the EUIImage seems to expect url for the image which it uses as a component. If we want to inline the SVG, we will need to be able to wrap it in the TSX file I believe. I am not sure what the process of doing this is though. I found this S/O post: https://stackoverflow.com/questions/59516405/not-able-to-import-svg-image-into-my-typescript-application. Should I use the same tool the post suggests or do we have a better mechanism? I need to add it inline to this block:
Specifically it is the steps diagram. |
I will raise a PR in a bit. |
Closing this issue since any last steps will be completed here: #1471 |
This issue to be transferred to corresponding repository
I am working on launching new light and dark mode themes provided by OUI component library for a target launch within 2.10. These changes support the vision expressed in Future Vision for Dashboards
I have identified the following front end related issues that prevent the theme from appearing complete and potential solutions within this feature:
EuiDescriptionList
to ensure correct stylingThe text was updated successfully, but these errors were encountered: