[Onboarding] - Refactor tabs and routing #5184
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,19 +1,30 @@ | ||
| import { HeaderLayout } from './layout/HeaderLayout'; | ||
| import PageContainer from '../../components/layout/components/PageContainer'; | ||
| import PageHeader from '../../components/layout/components/PageHeader'; | ||
| import { GetStartedTabs } from './components/get-started-tabs/GetStartedTabs'; | ||
| import { useAuthContext } from '../../components/providers/AuthProvider'; | ||
| import { Center, Loader } from '@mantine/core'; | ||
| import { colors } from '@novu/design-system'; | ||
| import { useTabSearchParams } from './components/get-started-tabs/useGetStartedTabs'; | ||
|
|
||
| const PAGE_TITLE = 'Get started'; | ||
|
|
||
| export function GetStartedPage() { | ||
| const { currentOrganization } = useAuthContext(); | ||
| const { currentTab, setTab } = useTabSearchParams(); | ||
|
|
||
|
There was a problem hiding this comment. this seems like should be used inside the
There was a problem hiding this comment. Thanks for the feedback! I will merge it as is for now, but we can memoize the tab behaviors so that they don't re-render the whole component tree if it becomes necessary. |
||
| return ( | ||
| <PageContainer title={PAGE_TITLE}> | ||
| <HeaderLayout> | ||
| <PageHeader title={PAGE_TITLE} /> | ||
| </HeaderLayout> | ||
| {currentOrganization ? ( | ||
| <GetStartedTabs currentTab={currentTab} setTab={setTab} /> | ||
| ) : ( | ||
| <Center> | ||
| <Loader color={colors.error} size={32} /> | ||
| </Center> | ||
| )} | ||
| </PageContainer> | ||
| ); | ||
| } | ||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,38 @@ | ||
| import { RingingBell, MultiChannel, Digest, HalfClock, Translation } from '@novu/design-system'; | ||
| import { CSSProperties } from 'react'; | ||
| import { OnboardingUseCasesTabsEnum } from '../../consts/OnboardingUseCasesTabsEnum'; | ||
|
|
||
| export interface GetStartedTabConfig { | ||
| value: OnboardingUseCasesTabsEnum; | ||
| icon: JSX.Element; | ||
| title: string; | ||
| } | ||
|
|
||
| const ICON_STYLE: Partial<CSSProperties> = { height: 20, width: 20, marginBottom: '12px' }; | ||
| export const TAB_CONFIGS: GetStartedTabConfig[] = [ | ||
| { | ||
| value: OnboardingUseCasesTabsEnum.IN_APP, | ||
| icon: <RingingBell style={ICON_STYLE} />, | ||
| title: 'In-app', | ||
| }, | ||
| { | ||
| value: OnboardingUseCasesTabsEnum.MULTI_CHANNEL, | ||
| icon: <MultiChannel style={ICON_STYLE} />, | ||
| title: 'Multi-channel', | ||
| }, | ||
| { | ||
| value: OnboardingUseCasesTabsEnum.DIGEST, | ||
| icon: <Digest style={ICON_STYLE} />, | ||
| title: 'Digest', | ||
| }, | ||
| { | ||
| value: OnboardingUseCasesTabsEnum.DELAY, | ||
| icon: <HalfClock style={ICON_STYLE} />, | ||
| title: 'Delay', | ||
| }, | ||
| { | ||
| value: OnboardingUseCasesTabsEnum.TRANSLATION, | ||
| icon: <Translation style={ICON_STYLE} />, | ||
| title: 'Translate', | ||
| }, | ||
| ]; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,63 +1,47 @@ | ||
| import { Container, Tabs } from '@mantine/core'; | ||
| import { Outlet } from 'react-router-dom'; | ||
| import { OnboardingUseCasesTabsEnum } from '../../consts/OnboardingUseCasesTabsEnum'; | ||
| import { UseCasesConst } from '../../consts/UseCases.const'; | ||
| import { GetStartedTab } from '../../layout/GetStartedTab'; | ||
| import { GetStartedTabConfig, TAB_CONFIGS } from './GetStartedTabs.const'; | ||
| import useStyles from './GetStartedTabs.style'; | ||
|
|
||
| interface IGetStartedTabsProps { | ||
| tabConfigs?: GetStartedTabConfig[]; | ||
| currentTab: OnboardingUseCasesTabsEnum; | ||
| setTab: (tab: OnboardingUseCasesTabsEnum) => void; | ||
| } | ||
|
|
||
| export const GetStartedTabs: React.FC<IGetStartedTabsProps> = ({ tabConfigs = TAB_CONFIGS, currentTab, setTab }) => { | ||
| const { classes } = useStyles(); | ||
|
|
||
| return ( | ||
| <Container fluid mt={15} ml={5}> | ||
| <Tabs | ||
| orientation="horizontal" | ||
| keepMounted={true} | ||
| onTabChange={(tabValue: OnboardingUseCasesTabsEnum) => { | ||
| setTab(tabValue); | ||
| }} | ||
| variant="default" | ||
| value={currentTab} | ||
| classNames={classes} | ||
| mb={15} | ||
| > | ||
| <Tabs.List> | ||
| {tabConfigs.map(({ value, icon, title }) => ( | ||
| <Tabs.Tab key={`tab-${value}`} value={value} icon={icon}> | ||
| {title} | ||
| </Tabs.Tab> | ||
| ))} | ||
| </Tabs.List> | ||
| {tabConfigs.map(({ value }) => ( | ||
| <Tabs.Panel key={`tab-panel-${value}`} value={value}> | ||
| {<GetStartedTab {...UseCasesConst[value]} />} | ||
| </Tabs.Panel> | ||
| ))} | ||
| </Tabs> | ||
| <Outlet /> | ||
| </Container> | ||
| ); | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,29 @@ | ||
| import { URLSearchParamsInit, useSearchParams } from 'react-router-dom'; | ||
| import { OnboardingUseCasesTabsEnum } from '../../consts/OnboardingUseCasesTabsEnum'; | ||
|
|
||
| const TAB_SEARCH_PARAM_NAME = 'tab'; | ||
| const DEFAULT_TAB: OnboardingUseCasesTabsEnum = OnboardingUseCasesTabsEnum.IN_APP; | ||
|
|
||
| interface GetStartedTabSearchParams { | ||
| [TAB_SEARCH_PARAM_NAME]: OnboardingUseCasesTabsEnum; | ||
| } | ||
|
|
||
| const DEFAULT_PARAMS: GetStartedTabSearchParams = { | ||
| [TAB_SEARCH_PARAM_NAME]: DEFAULT_TAB, | ||
| } satisfies URLSearchParamsInit; | ||
|
|
||
| export const useTabSearchParams = () => { | ||
| const [params, setParams] = useSearchParams(DEFAULT_PARAMS as unknown as URLSearchParamsInit); | ||
|
|
||
| const setTab = (tab: OnboardingUseCasesTabsEnum) => { | ||
| // replace is used so that changing the search params isn't considered a change in page | ||
| setParams({ [TAB_SEARCH_PARAM_NAME]: tab }, { replace: true }); | ||
| }; | ||
|
|
||
| const currentTab = (params.get(TAB_SEARCH_PARAM_NAME) as OnboardingUseCasesTabsEnum) ?? DEFAULT_TAB; | ||
|
|
||
| return { | ||
| currentTab, | ||
| setTab, | ||
| }; | ||
| }; |
File renamed without changes.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,16 +1,15 @@ | ||
| import { OnboardingUseCases } from './types'; | ||
| import { InAppUseCaseConst } from './InAppUseCase.const'; | ||
| import { MultiChannelUseCaseConst } from './MultiChannelUseCase.const'; | ||
| import { DelayUseCaseConst } from './DelayUseCase.const'; | ||
| import { TranslationUseCaseConst } from './TranslationUseCase.const'; | ||
| import { DigestUseCaseConst } from './DigestUseCase.const'; | ||
| import { OnboardingUseCasesTabsEnum } from './OnboardingUseCasesTabsEnum'; | ||
|
|
||
| export const UseCasesConst: OnboardingUseCases = { | ||
| [OnboardingUseCasesTabsEnum.IN_APP]: InAppUseCaseConst, | ||
| [OnboardingUseCasesTabsEnum.MULTI_CHANNEL]: MultiChannelUseCaseConst, | ||
| [OnboardingUseCasesTabsEnum.DELAY]: DelayUseCaseConst, | ||
| [OnboardingUseCasesTabsEnum.TRANSLATION]: TranslationUseCaseConst, | ||
| [OnboardingUseCasesTabsEnum.DIGEST]: DigestUseCaseConst, | ||
| }; |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This conditional (that requires a re-render) is why search params don't persist across refresh. Once we remove this, they will work as expected
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not related to this scope
I wonder if we could/want to create a "loading" indication on our FF's so in such cases we would create a loading state, so we would avoid unnecessary rerenders and screen changes.
@antonjoel82 would love to hear your thoughts on it :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@djabarovgeorge What is
FFin this context?Sounds like a good UX improvement! We could do a temporary one, but overall my preference would be to upgrade our React version to use
Suspensewhich will be a huge improvement overall.Alternatively, one way I've done this in the past to avoid the re-routing issue is to route to a container page (just a page component that uses the hook to determine which content to show). Then, we'd delete that page later and point directly to the new functionality
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry by i meant feature flag by FF.
Still had not the chance to use Suspense wonder how good is it.
Agree that a middleware component could help.
thanks for the feedback 🙃