[Workspace]Dismiss get started for search/essential/analytics overview page #8874
Conversation
❌ Empty Changelog SectionThe Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8874 +/- ##
==========================================
+ Coverage 61.02% 61.04% +0.01%
==========================================
Files 3813 3814 +1
Lines 91401 91472 +71
Branches 14443 14453 +10
==========================================
+ Hits 55782 55842 +60
- Misses 32056 32067 +11
Partials 3563 3563
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Hailong-am
requested review from
ananzh,
kavilla,
AMoo-Miki,
ashwin-pc,
joshuarrrr,
abbyhu2000,
zengyan-amazon,
zhongnansu,
manasvinibs,
ZilongX,
Flyingliuhub,
curq,
bandinib-amzn,
SuZhou-Joe,
ruanyl,
BionIT,
xinruiba,
zhyuanqi,
mengweieric,
LDrago27,
virajsanghvi,
sejli and
joshuali925
as code owners
Signed-off-by: Hailong Cui <[email protected]>
Signed-off-by: Hailong Cui <[email protected]>
Hailong-am
force-pushed
the
dismiss-getstarted-card
branch
from
d0d13a9 to
8b27d2d
Compare
Hailong-am
changed the title
| } = useOpenSearchDashboards<CoreStart>(); | ||
| const [isPopoverOpen, setIsPopoverOpen] = useState(false); | ||
| const [isGetStartedDismissed, setIsGetStartedDismissed] = useState<boolean>( | ||
| !!uiSettings.get(SEARCH_WORKSPACE_DISMISS_GET_STARTED) |
There was a problem hiding this comment.
Nit: I remember uiSettings has a get$ method that can return an observable, maybe we can use that and there is no need to have a state here.
There was a problem hiding this comment.
the state isGetStartedDismissed used for multiple place and we need a variable for
const contextMenuItems = [
{
name: isGetStartedDismissed ? show : hide,
icon: <EuiIcon type={isGetStartedDismissed ? 'eye' : 'eyeClosed'} />,
onClick: async () => {
await dismissGetStartCards(!isGetStartedDismissed);
closePopover();
},
},
];
There was a problem hiding this comment.
const isGetStartedDismissed = useObservable<boolean>(!!uiSettings.get$(SEARCH_WORKSPACE_DISMISS_GET_STARTED), may work as well and setIsGetStartedDismissed seems useless.
src/plugins/workspace/public/components/workspace_use_case_overview_app.tsx
Show resolved
Hide resolved
| @@ -28,3 +29,14 @@ export const uiSettings: Record<string, UiSettingsParams> = { | |||
| requiresPageReload: true, | |||
| }, | |||
| }; | |||
|
|
|||
| export const searchOverviewPageUISetting: Record<string, UiSettingsParams> = { | |||
| [SEARCH_WORKSPACE_DISMISS_GET_STARTED]: { | |||
There was a problem hiding this comment.
Instead of ui settings, shall we simply store it in local storage? I feel local storage makes more sense to me for a UI behavior
There was a problem hiding this comment.
we want to persist the settings and even if when they switch browser and devices.
| } | ||
| }, [isGetStartedDismissed, page]); | ||
|
|
||
| const hide = i18n.translate('home.searchOverview.getStartedCard.setting.hide.label', { |
There was a problem hiding this comment.
Nit: since show is just text, maybe name it like hideGetStartedLabel sound more appropriate?
| defaultMessage: 'Hide Get started with Search', | ||
| }); | ||
|
|
||
| const show = i18n.translate('home.searchOverview.getStartedCard.setting.show.label', { |
Description
Add setting button for search overview page to let user to dismiss get started section
Issues Resolved
Screenshot
Testing the changes
Changelog
Check List
yarn test:jestyarn test:jest_integration