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

Theming - Preload variable improvements #1679

Open
bmingles opened this issue Dec 8, 2023 · 0 comments
Open

Theming - Preload variable improvements #1679

bmingles opened this issue Dec 8, 2023 · 0 comments
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@bmingles
Copy link
Contributor

bmingles commented Dec 8, 2023

Certain UI elements are visible before theming plugins can finish loading. e.g. in DHC, the background color and loading spinner. In DHE everything in the login form. To improve the user experience, we are storing certain "preload" variables in localStorage and then loading them on next page load independent of the plugins. Preload variable names + default values are currently stored in ThemeModel DEFAULT_PRELOAD_DATA_VARIABLES.

There's a couple of things that don't quite feel right with the current implementation.

Variables for the login page + animation technically shouldn't be needed for DHC preload, but they are needed for DHE.

| '--dh-color-login-form-bg'
| '--dh-color-login-status-message'
| '--dh-color-login-logo-bg'
| '--dh-color-login-footer-fg'
| '--dh-color-random-area-plot-animation-fg-fill'
| '--dh-color-random-area-plot-animation-fg-stroke'
| '--dh-color-random-area-plot-animation-bg'
| '--dh-color-random-area-plot-animation-grid';

It would be nice to have DHE be responsible for these, but this will require a way for apps to provide preload variables that they need

Also, @mofojed has called out that it seems odd for component specific variables to be declared in the preload as opposed to more generic semantic variables. We should think through this one to see if there is a better approach.

@bmingles bmingles added enhancement New feature or request triage Issue requires triage labels Dec 8, 2023
@bmingles bmingles self-assigned this Dec 8, 2023
@vbabich vbabich added this to the Backlog milestone Dec 12, 2023
@vbabich vbabich removed the triage Issue requires triage label Dec 12, 2023
bmingles added a commit that referenced this issue Jan 11, 2024
- Preload now always applies default preload values before applying
cached ones. This ensures that cached preload values don't prevent new
defaults from being applied if / when we add new variables to the
preload list
- Default preload variables can now be passed in to ThemeUtils +
ThemeProvider. This will allow DHE to specify additional variables if
needed

resolves #1695 and part of #1679
mofojed pushed a commit to deephaven/deephaven-core that referenced this issue Jan 17, 2024
# [0.59.0](deephaven/web-client-ui@v0.58.0...v0.59.0) (2024-01-17)


### Bug Fixes

* GoToRow timestamp fails when selected row is out of view ([#1717](deephaven/web-client-ui#1717)) ([9ddc973](deephaven/web-client-ui@9ddc973)), closes [#1561](deephaven/web-client-ui#1561)
* Interface for IrisGridTableModelTemplate.backgroundColorForCell ([#1699](deephaven/web-client-ui#1699)) ([73e1837](deephaven/web-client-ui@73e1837)), closes [#1697](deephaven/web-client-ui#1697)
* Moved logos so they show in production build ([#1713](deephaven/web-client-ui#1713)) ([a3bea73](deephaven/web-client-ui@a3bea73)), closes [#1712](deephaven/web-client-ui#1712)
* re-colorize command codeblocks when theme changes ([#1731](deephaven/web-client-ui#1731)) ([b1e42f5](deephaven/web-client-ui@b1e42f5))
* TimeInput not triggering onChange on incomplete values ([#1711](deephaven/web-client-ui#1711)) ([6894d96](deephaven/web-client-ui@6894d96)), closes [#1710](deephaven/web-client-ui#1710)


### Features

* Action button tooltips ([#1706](deephaven/web-client-ui#1706)) ([bff6bf9](deephaven/web-client-ui@bff6bf9)), closes [#1705](deephaven/web-client-ui#1705)
* Add support for useDeferredApi ([#1725](deephaven/web-client-ui#1725)) ([51ebe1b](deephaven/web-client-ui@51ebe1b))
* Improved preload variable handling ([#1723](deephaven/web-client-ui#1723)) ([ed41c42](deephaven/web-client-ui@ed41c42)), closes [#1695](deephaven/web-client-ui#1695) [#1679](deephaven/web-client-ui#1679)
* NavTabList component ([#1698](deephaven/web-client-ui#1698)) ([96641fb](deephaven/web-client-ui@96641fb))
* Reject promise immediately if var not found ([#1718](deephaven/web-client-ui#1718)) ([43d40bd](deephaven/web-client-ui@43d40bd)), closes [#1701](deephaven/web-client-ui#1701)
* theming tweaks ([#1727](deephaven/web-client-ui#1727)) ([f919a7e](deephaven/web-client-ui@f919a7e))


### BREAKING CHANGES

* - Subclasses of IrisGridTableModelTemplate or it's subclasses that use
backgroundColorForCell may need to update their signature to accept the
theme if they are calling the superclass


Co-authored-by: deephaven-internal <deephaven-internal@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants