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

[Next theme] Add modal to notify users of theme updates #4715

Merged
merged 11 commits into from
Aug 31, 2023

Conversation

joshuarrrr
Copy link
Member

@joshuarrrr joshuarrrr commented Aug 11, 2023

Description

Copy and images provisional, for demo purposes only.

Add a new modal to the home page that provides explanation of theme changes and a link to advanced settings where themes can be managed.

On close or dismissal, set a localstorage value that will hide the modal on future visits to home.

Also provide a config value that can be used to permanently disable the modal.

Issues Resolved

fixes #4676

Screenshot

Testing the changes

  1. Modal should appear when navigating to home or first loading the application
  2. After dismissing or closing the modal, it should not reappear, unless localstorage is cleared
  3. Setting config value should be able to hide modal regardless of localstorage settings
  4. Should not otherwise interfere with welcome page or other home page conditional rendering
  5. Clicking the link to advanced settings should navigate via SPA

Check List

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

Copy and images provisional, for demo purposes only.

Signed-off-by: Josh Romero <[email protected]>
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #4715 (ec1f7b9) into main (03ae2bd) will decrease coverage by 0.01%.
Report is 2 commits behind head on main.
The diff coverage is 54.54%.

@@            Coverage Diff             @@
##             main    #4715      +/-   ##
==========================================
- Coverage   66.46%   66.46%   -0.01%     
==========================================
  Files        3390     3391       +1     
  Lines       64742    64753      +11     
  Branches    10312    10312              
==========================================
+ Hits        43030    43036       +6     
- Misses      19169    19174       +5     
  Partials     2543     2543              
Flag Coverage Δ
Linux_1 34.87% <ø> (ø)
Linux_2 55.15% <ø> (ø)
Linux_3 44.18% <ø> (+<0.01%) ⬆️
Linux_4 35.05% <54.54%> (+<0.01%) ⬆️
Windows_1 34.88% <ø> (ø)
Windows_2 55.12% <ø> (ø)
Windows_3 44.18% <ø> (-0.01%) ⬇️
Windows_4 35.05% <54.54%> (+<0.01%) ⬆️

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

Files Changed Coverage Δ
src/plugins/home/config.ts 100.00% <ø> (ø)
.../public/application/components/new_theme_modal.tsx 33.33% <33.33%> (ø)
...plugins/home/public/application/components/home.js 87.27% <62.50%> (-4.22%) ⬇️

@joshuarrrr joshuarrrr changed the title Feat/next preview notify [Next theme] Add modal to notify users of theme updates Aug 29, 2023
@joshuarrrr joshuarrrr requested a review from curq as a code owner August 29, 2023 17:54
@joshuarrrr
Copy link
Member Author

@ashwin-pc
Copy link
Member

@joshuarrrr marking this as draft until its ready for review. Feel free to unmark it if thats not correct.

@ashwin-pc ashwin-pc marked this pull request as draft August 29, 2023 23:45
and hide modal if v7 theme is in use

Signed-off-by: Josh Romero <[email protected]>
@joshuarrrr joshuarrrr marked this pull request as ready for review August 30, 2023 19:00
<EuiText>
<FormattedMessage
id="home.newThemeModal.previewDescription.previewDetail"
defaultMessage="You are now previewing the newest OpenSearch Dashboards theme with improved light and dark
Copy link
Collaborator

@AMoo-Miki AMoo-Miki Aug 30, 2023

Choose a reason for hiding this comment

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

"newest" sounds odd to me; it reads as "previewing the latest new theme". why not just "latest" or just "new"?

@ashwin-pc ashwin-pc merged commit b846e80 into opensearch-project:main Aug 31, 2023
opensearch-trigger-bot bot pushed a commit that referenced this pull request Aug 31, 2023
* Feat (home): Add modal to introduce `next` theme changes

Copy and images provisional, for demo purposes only.

Signed-off-by: Josh Romero <[email protected]>

* revert unintended config change

Signed-off-by: Josh Romero <[email protected]>

* add changelog

Signed-off-by: Josh Romero <[email protected]>

* Disable new theme modal for functional tests

Signed-off-by: Josh Romero <[email protected]>

* add deep link to settings

and hide modal if v7 theme is in use

Signed-off-by: Josh Romero <[email protected]>

* add unit tests and fix imports

Signed-off-by: Josh Romero <[email protected]>

---------

Signed-off-by: Josh Romero <[email protected]>
(cherry picked from commit b846e80)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md
kavilla pushed a commit that referenced this pull request Sep 1, 2023
* Feat (home): Add modal to introduce `next` theme changes

Copy and images provisional, for demo purposes only.

Signed-off-by: Josh Romero <[email protected]>

* revert unintended config change

Signed-off-by: Josh Romero <[email protected]>

* add changelog

Signed-off-by: Josh Romero <[email protected]>

* Disable new theme modal for functional tests

Signed-off-by: Josh Romero <[email protected]>

* add deep link to settings

and hide modal if v7 theme is in use

Signed-off-by: Josh Romero <[email protected]>

* add unit tests and fix imports

Signed-off-by: Josh Romero <[email protected]>

---------

Signed-off-by: Josh Romero <[email protected]>
(cherry picked from commit b846e80)
Signed-off-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>

# Conflicts:
#	CHANGELOG.md

Co-authored-by: github-actions[bot] <github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inform users of theme "Next" preview
3 participants