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

feat: Theme Selector #1661

Merged
merged 13 commits into from
Dec 6, 2023
Merged

feat: Theme Selector #1661

merged 13 commits into from
Dec 6, 2023

Conversation

bmingles
Copy link
Contributor

@bmingles bmingles commented Nov 29, 2023

  • Added a theme selector to settings menu when there are multiple themes available
  • IrisGrid and Monaco now respond dynamically to selected theme change
  • Misc moving of theme utils
  • Added vite config for local plugin dev

resolves #1660

Copy link

codecov bot commented Nov 29, 2023

Codecov Report

Attention: 40 lines in your changes are missing coverage. Please review.

Comparison is base (1e40d3e) 46.65% compared to head (2fd804c) 46.63%.
Report is 2 commits behind head on main.

❗ Current head 2fd804c differs from pull request most recent head a4597dc. Consider uploading reports for the commit a4597dc to get more accurate results

Files Patch % Lines
packages/console/src/monaco/MonacoUtils.ts 0.00% 10 Missing ⚠️
packages/iris-grid/src/IrisGridThemeProvider.tsx 11.11% 8 Missing ⚠️
packages/components/src/theme/ThemePicker.tsx 0.00% 7 Missing ⚠️
...ackages/console/src/monaco/MonacoThemeProvider.tsx 0.00% 7 Missing ⚠️
packages/code-studio/src/settings/SettingsMenu.tsx 16.66% 5 Missing ⚠️
packages/iris-grid/src/IrisGrid.tsx 25.00% 3 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1661      +/-   ##
==========================================
- Coverage   46.65%   46.63%   -0.02%     
==========================================
  Files         599      602       +3     
  Lines       36671    36718      +47     
  Branches     9192     9204      +12     
==========================================
+ Hits        17109    17124      +15     
- Misses      19510    19542      +32     
  Partials       52       52              
Flag Coverage Δ
unit 46.63% <46.66%> (-0.02%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@bmingles bmingles marked this pull request as ready for review November 30, 2023 19:10
@bmingles bmingles requested a review from mofojed November 30, 2023 19:12
Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor cleanup, looks good overall

README.md Outdated Show resolved Hide resolved
packages/code-studio/src/styleguide/ThemeColors.tsx Outdated Show resolved Hide resolved
packages/grid/src/GridThemeContext.ts Outdated Show resolved Hide resolved
Comment on lines +53 to +55
const themePluginEntries = [...pluginMap.entries()].filter(
(entry): entry is [string, ThemePlugin] => isThemePlugin(entry[1])
);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you can shorten it, you're not using the keys at all:

Suggested change
const themePluginEntries = [...pluginMap.entries()].filter(
(entry): entry is [string, ThemePlugin] => isThemePlugin(entry[1])
);
const themePluginEntries = [...pluginMap.values()].filter(isThemePlugin);

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The keys aren't used here, but they do get consumed in the next block when themePluginEntries is iterated, so the key + value needs to be present

@bmingles bmingles force-pushed the 1660-theme-selector branch from a097d86 to 9f4f3aa Compare December 6, 2023 16:14
@bmingles bmingles requested a review from mofojed December 6, 2023 16:18
@bmingles bmingles merged commit 5e2be64 into deephaven:main Dec 6, 2023
4 checks passed
@bmingles bmingles deleted the 1660-theme-selector branch December 6, 2023 16:41
@github-actions github-actions bot locked and limited conversation to collaborators Dec 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Theming - Theme selector
2 participants