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

Updates to Advanced Settings for theme Next #4330

Closed
Tracked by #4675
KrooshalUX opened this issue Jun 20, 2023 · 15 comments
Closed
Tracked by #4675

Updates to Advanced Settings for theme Next #4330

KrooshalUX opened this issue Jun 20, 2023 · 15 comments
Assignees
Labels
next-theme OUI compliance Issues and PRs to maximize OUI usage and remove style and component hacks v2.11.0

Comments

@KrooshalUX
Copy link

KrooshalUX commented Jun 20, 2023

In support of new options to control the look & feel of OpenSearch Dashboards, I would like to introduce a new controls within Advanced Settings. The north star "ideal" change for Advacned Settings would include introducing the "Appearance" category. At a later time, we will start to reorganize other settings and disambiguate "General" as it currently contains an unmanageable/unscannable and somewhat disorganized set of controls that could be grouped into smaller, more consumable settings types.

North star proposal

Appearance is added to the category list (the expression of which depends on the status of this issue) after General, and replace "Accessibility" - as there is currently only 1 accessibility setting which also is related to "Appearance".

These settings would contain the following as cherry-picked from General and Accessibility - as well as new options, marked:

  • Light/Dark Mode selection using OuiRadioCard - (replaces OuiSwitch in current Dark Mode). As per Switch default theme to "Next" from "V7" #4677 - for upgrading users, the radio button should reflect the previously selected mode.
    - Light Mode
    - Dark Mode
    - Automatic - this control utilizes browser settings to switch theme modes. The theme mode change will take place on page change (as in: if the browser setting dictates that at 5pm, the users browser experience switches into dark mode, do not prompt the user to refresh the page to switch into dark mode.)
    - At a later time this change should apply without relying on page change.
  • Side nav style (existing)
  • Disable Animations (existing, currently in "Accessibility" category)

Design:
image

Alternate options

  • Without introducing "Appearance" as a category within Advacned settings, provide the following change to the "General" category:
  • Rename "Dark mode" section to "Theme mode". The primary reason for this change is to support [Look & Feel] Add dark mode option that respects browser settings #4462
  • Change dark mode toggle into OuiRadioCard group with Light, Dark and Automatic options
  • Introduce theme picker to Advanced Settings above "Theme mode" section
  • Add a link to theme feedback (forum post URL) in the description of the theme switcher
@ananzh ananzh added OUI compliance Issues and PRs to maximize OUI usage and remove style and component hacks and removed untriaged labels Jun 20, 2023
@joshuarrrr joshuarrrr removed their assignment Jun 20, 2023
@KrooshalUX KrooshalUX changed the title Introduce 'Appearance' as category within Advanced Settings Introduce 'Appearance' as category within Advanced Settings & increase light/dark mode management features Jun 20, 2023
@ashwin-pc
Copy link
Member

Should we also expose the compressed header as a toggle here? I only ask because we seem to be offering the sidenav as an option

@kgcreative
Copy link
Member

kgcreative commented Jun 23, 2023

A couple questions/comments:

@KrooshalUX:

  1. light/dark mode: Should this be "automatic" by default, instead of dark or light?
  2. side nav style -- should this be eventually deprecated? I'd love to see what the actual outcome of changing this setting is

@ashwin-pc + @KrooshalUX :

  1. I wouldn't mind seeing compressed header be part of the appearance toggles.
  2. I also wouldn't mind seeing more custom branding settings be moved to appearance
  3. I would love to see any number of .yml settings be actually managed here. In the fullness of time, we should audit which settings we should toggle via .yml, and which settings make sense to bring here. The more we don't force full application restarts and the more we can hot reload, the better, IMO

@KrooshalUX
Copy link
Author

KrooshalUX commented Jun 23, 2023

@kgcreative @ashwin-pc

  • These changes are related to the scope of 2.10. If we feel like we can comfortably increase scope to add the compressed header toggle here as well, I can add it. The side nav toggle you see is existing functionality, I am just reorganizing it into the new Appearance category. The only net-new functionality is the light/dark/automatic toggle. Please confirm from engineering end first @joshuarrrr
  • We still need to discuss light/dark/automatic by default. There is a promise of dark mode by default for 2.10 that I think introduces risk related to vis colors, but I would like to have that discussion in the theming issue rather than here, as this issue scope is about introducing the controls and I don't want to get anyone blocked on starting. I am working on updates to the theming issue right now.
  • Side nav style - Are you asking about the toggle for "side nav style" or the way that OuiSideNav is used on this page?
    - If toggle - I don't know enough about the legacy usage to rip this out in 2.10. Perhaps in another release we can tackle deprecating it, but this is just a light re-org for now to build the "Appearance" category and put existing like-with-like.
    - If you are referencing the use of OuiSideNav on this page - we dont have plans in the short term to deprecate the use of OuiSideNav - however, in the longer term in the lens of working toward Future Vision for Dashboards we will modify OuiCollapsableNav to include nested children similar to OuiSideNav/ OuiTreeView (doesn't currently support) and this is also a change to the OSD API that allows plugins to register pages to OuiCollapsableNav. In fact, I have another issue open to move the Advanced Settings category navigation (currently utilizing search bar filters rather than using tree inside the side nav).
  • Re: custom styles - Since I only have the context of what is available in playground.opensearch.org, can you provide information (github issue, screenshots) of what the current custom branding functionality is? If it is existing functionality I am happy to move it into Appearance. If its net-new, lets discuss it for a future release.
  • Re: Additional YML toggles - We are discussing another attempt at a YML audit to introduce UI for , but I believe that is out of scope for 2.10 and I will be posting a research issue about it shortly.

@ashwin-pc
Copy link
Member

Im good with the incremental move, thanks for clarifying that

@wbeckler
Copy link

wbeckler commented Jul 7, 2023

Are advanced settings accessible to all users or just to the admins? If it's just admins, should the appearance settings live in a different place accessible to individual users, maybe stored in saved objects?

@kgcreative
Copy link
Member

kgcreative commented Jul 7, 2023

@wbeckler -- today advanced settings are accessible to all users, although not all users have permission to update them.

Additionally, if multitenancy is enabled, advanced settings only apply to your currently selected tenant. This is because advanced settings is a saved object, which co-exists in the .kibana index along with all other saved objects.

We have identified a need for disambiguating between application (opensearch dashboards) settings, tenant (and down the road, project) settings, and finally user settings.

My understanding is that today the OSD security plugin only provides role-based controls in OSD, and, there is no concept of a user, or a user object therefore, some settings are saved to local storage or are session based (such as recent pages, your last selected tenant, etc), but until we have a user object to save configuration options to, we are unable to move those items forward.

Some appearance settings should be user (instead of tenant or application). For example, today, if I change the theme, or change dark/light mode, those settings apply to every user in the current tenant, or to every user in dashboards.

@wbeckler
Copy link

wbeckler commented Jul 8, 2023

Given that, shouldn't light/dark mode exist somewhere other than advanced settings, which always apply tenant-wide or dashboard-wide?

@kgcreative
Copy link
Member

It should in the long term, but that has dependencies. I think this issue is incremental to the current experience, so it just organizes some of the appearace settings that already exist into a category where they are grouped together.

@KrooshalUX
Copy link
Author

KrooshalUX commented Jul 10, 2023

@wbeckler yes, this is a risk that was called out at the beginning of the theme change project (although we could have done a better job at github record keeping). We are in agreement that there needs to be a better end-user mechanism for manipulating light/dark mode selection (though there is a case to be made against end users choosing themes, especially in cases of custom branding). Perhaps instead of "one or the other" type solution, we provide the controls in two places - my initial thought was to add it as a quick-control to the user menu in the application top bar. We will still need to address permissions / security plugin issues with this.

Two additional points:

  • With the knowledge that users do not have explicit control over their light/dark mode settings (among other risks) I will be updating the Dark Mode by default issue as a change like this can have significant impact on end users who do not have permissions to change to light mode
  • The scope of this UI change to advanced settings is 1) based on replacing the base sass variables within OUI, which we have now pivoted away from to provide customers both the original light and dark as well as the "next" light and dark themes (which I am very happy about) and 2) too large for 2.10 and therefore I will be posting a p0 version of this UI and we can count this UI as a "north star" for a following improvement to advanced settings.

@KrooshalUX KrooshalUX changed the title Introduce 'Appearance' as category within Advanced Settings & increase light/dark mode management features Updates to Advanced Settings for theme `Next1 Aug 3, 2023
@KrooshalUX KrooshalUX changed the title Updates to Advanced Settings for theme `Next1 Updates to Advanced Settings for theme Next Aug 3, 2023
@KrooshalUX
Copy link
Author

@joshuarrrr I have updated this issue with the north star design and alternate possibilities for the short term

@KrooshalUX
Copy link
Author

@joshuarrrr @wbeckler can I get confirmation on whether 'automatic' switch is in scope for 2.10? This would help with documentation and messaging and I can also firm up the details in the short term proposal included in the issue.

@joshuarrrr
Copy link
Member

These changes are related to the scope of 2.10. If we feel like we can comfortably increase scope to add the compressed header toggle here as well, I can add it. The side nav toggle you see is existing functionality, I am just reorganizing it into the new Appearance category. The only net-new functionality is the light/dark/automatic toggle. Please confirm from engineering end first @joshuarrrr

Yes, I can confirm.

@joshuarrrr
Copy link
Member

@joshuarrrr @wbeckler can I get confirmation on whether 'automatic' switch is in scope for 2.10? This would help with documentation and messaging and I can also firm up the details in the short term proposal included in the issue.

No, automatic switching is not in scope for 2.10.

@kavilla
Copy link
Member

kavilla commented Sep 5, 2023

@joshuarrrr will split off the remaining work that is targetted for 2.11. Completed all work related to 2.10.

@joshuarrrr joshuarrrr added v2.11.0 and removed v2.10.0 labels Sep 5, 2023
@joshuarrrr
Copy link
Member

Wrapping this up for now, based on what we delivered for the 2.10 release:

  1. We created the new "Appearance" category (and moved relevant setting there) and removed the "Accessibility" category
  2. We made category headers deep-linkable
  3. We added a forum feedback link

Not completed:

  • Change dark mode toggle into OuiRadioCard group with Light, Dark and Automatic options

I'll close this issue for now, as other improvements down the line will likely warrant revised designs, and we can reopen if needed.


At a later time, we will start to reorganize other settings and disambiguate "General" as it currently contains an unmanageable/unscannable and somewhat disorganized set of controls that could be grouped into smaller, more consumable settings types.

There's not really a "General" category per se; any setting that doesn't specify a category will show up there, so we can definitely assign categories to existing settings at any point.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
next-theme OUI compliance Issues and PRs to maximize OUI usage and remove style and component hacks v2.11.0
Projects
Status: Done
Development

No branches or pull requests

7 participants