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

[Manual Backport 2.x] refactor: simplify theme configuration and defaulting (#7625) #7746

Merged

Conversation

virajsanghvi
Copy link
Collaborator

Backport from #7625


Signed-off-by: Viraj Sanghvi [email protected]
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit bcdbbef)

Description

The backport failed because user specific themes was pulled from 2.x. Some feedback says it was due to the UX, #6978 says due to testing, and others because it caused a blank white screen. Because I'm not sure that'll get resolved, I changed my PR to handle the differences.

The main callouts are that:

  • The changes to ui_render_mixin and the bootstrap and startup template files are now just within the ui_render_mixin and bootstrap file. Some of these changes came from the render view template, and for now, I just made sure the themeVersion is correct there (when I add a new theme, will correct differences in fonts file).
  • I added docs/theme.md even though the base isn't present as it still seems valid
  • The startup template was never reverted from 2.x, and so it doesn't do anything, and I just left my changes on top.
  • The header menu is still present as it was recently introduced even though I don't think it does anything - similarly, I just left my changes on top

Issues Resolved

N/A

Screenshot

N/A

Testing the changes

I was able to switch between themes locally, validated correct css files load. Pages look correct and no errors.

Changelog

  • skip

Check List

  • All tests pass
    • yarn test:jest
    • yarn test:jest_integration
  • New functionality includes testing.
  • New functionality has been documented.
  • Update CHANGELOG.md
  • Commits are signed per the DCO using --signoff

…ject#7625)

---------

Signed-off-by: Viraj Sanghvi <[email protected]>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>
(cherry picked from commit bcdbbef)
@github-actions github-actions bot added the Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry label Aug 18, 2024
Copy link

codecov bot commented Aug 18, 2024

Codecov Report

Attention: Patch coverage is 85.36585% with 6 lines in your changes missing coverage. Please review.

Project coverage is 63.82%. Comparing base (b3078c4) to head (9e44be1).
Report is 6 commits behind head on 2.x.

Files Patch % Lines
src/legacy/ui/ui_render/ui_render_mixin.js 0.00% 3 Missing ⚠️
packages/osd-ui-shared-deps/theme.ts 60.00% 0 Missing and 2 partials ⚠️
src/core/public/ui_settings/ui_settings_client.ts 88.88% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##              2.x    #7746      +/-   ##
==========================================
+ Coverage   63.81%   63.82%   +0.01%     
==========================================
  Files        3646     3647       +1     
  Lines       80751    80781      +30     
  Branches    12842    12844       +2     
==========================================
+ Hits        51528    51556      +28     
- Misses      26067    26070       +3     
+ Partials     3156     3155       -1     
Flag Coverage Δ
Linux_1 29.91% <63.15%> (+0.03%) ⬆️
Linux_2 55.98% <88.88%> (+0.04%) ⬆️
Linux_3 40.54% <60.00%> (+0.02%) ⬆️
Linux_4 31.49% <63.15%> (+0.02%) ⬆️
Windows_1 29.93% <63.15%> (+0.03%) ⬆️
Windows_2 55.93% <88.88%> (+0.04%) ⬆️
Windows_3 40.54% <60.00%> (+0.02%) ⬆️
Windows_4 31.49% <63.15%> (+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.

@AMoo-Miki AMoo-Miki merged commit 1e5addb into opensearch-project:2.x Aug 19, 2024
65 of 66 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip-Changelog PRs that are too trivial to warrant a changelog or release notes entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants