samtfm commented on code in PR #21033:
URL: https://github.com/apache/superset/pull/21033#discussion_r942717817
##########
superset-frontend/src/views/CRUD/welcome/Welcome.tsx:
##########
@@ -179,7 +179,10 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
setItem(LocalStorageKeys.homepage_collapse_state, state);
};
+ const WelcomeMessageExtension = extensionsRegistry.get('welcome.message');
const WelcomeTopExtension = extensionsRegistry.get('welcome.banner');
+ const WelcomeDataExtension = extensionsRegistry.get('welcome.data');
+ const WelcomeTableExtension = extensionsRegistry.get('welcome.table');
Review Comment:
I think the convention for these names is they should refer mostly to the
place in the layout that the extension will go, without referring to the
content that Preset will be putting into the extension.
Ideally, these new components could fit under a single extension point,
maybe something like "welcome.main.replacement" since it's replacing the body
content of the welcome page. The details that Preset uses this extension point
to add extra data and a table isn't really relevant to the superset side.
##########
superset-frontend/src/views/CRUD/welcome/Welcome.tsx:
##########
@@ -282,71 +285,87 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
!activityData?.Examples && !activityData?.Viewed;
return (
<WelcomeContainer>
+ {WelcomeMessageExtension && <WelcomeMessageExtension user={user} />}
{WelcomeTopExtension && <WelcomeTopExtension />}
- <WelcomeNav>
- <h1 className="welcome-header">Home</h1>
- {isFeatureEnabled(FeatureFlag.THUMBNAILS) ? (
- <div className="switch">
- <AntdSwitch checked={checked} onChange={handleToggle} />
- <span>Thumbnails</span>
- </div>
- ) : null}
- </WelcomeNav>
- <Collapse activeKey={activeState} onChange={handleCollapse} ghost bigger>
- <Collapse.Panel header={t('Recents')} key="1">
- {activityData &&
- (activityData.Viewed ||
- activityData.Examples ||
- activityData.Created) &&
- activeChild !== 'Loading' ? (
- <ActivityTable
- user={{ userId: user.userId! }} // user is definitely not a
guest user on this page
- activeChild={activeChild}
- setActiveChild={setActiveChild}
- activityData={activityData}
- loadedCount={loadedCount}
- />
- ) : (
- <LoadingCards />
- )}
- </Collapse.Panel>
- <Collapse.Panel header={t('Dashboards')} key="2">
- {!dashboardData || isRecentActivityLoading ? (
- <LoadingCards cover={checked} />
- ) : (
- <DashboardTable
- user={user}
- mine={dashboardData}
- showThumbnails={checked}
- examples={activityData?.Examples}
- />
- )}
- </Collapse.Panel>
- <Collapse.Panel header={t('Charts')} key="3">
- {!chartData || isRecentActivityLoading ? (
- <LoadingCards cover={checked} />
- ) : (
- <ChartTable
- showThumbnails={checked}
- user={user}
- mine={chartData}
- examples={activityData?.Examples}
- />
- )}
- </Collapse.Panel>
- <Collapse.Panel header={t('Saved queries')} key="4">
- {!queryData ? (
- <LoadingCards cover={checked} />
- ) : (
- <SavedQueries
- showThumbnails={checked}
- user={user}
- mine={queryData}
- featureFlag={isFeatureEnabled(FeatureFlag.THUMBNAILS)}
- />
- )}
- </Collapse.Panel>
- </Collapse>
+ {WelcomeDataExtension && (
+ <WelcomeDataExtension examples={activityData?.Examples} user={user} />
+ )}
+ {WelcomeTableExtension && <WelcomeTableExtension user={user} />}
Review Comment:
If possible, I think it would be best to avoid passing props to extensions
to leave them more general purpose.
That said, having extra unused props shouldn't really cause issues in
someone else's extension, so some amount of this pattern is probably fine if
there isn't a clean way to get this data from hooks inside the component.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
