samtfm commented on code in PR #20857:
URL: https://github.com/apache/superset/pull/20857#discussion_r937223411
##########
superset-frontend/src/views/CRUD/welcome/Welcome.tsx:
##########
@@ -180,6 +180,8 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
};
const WelcomeTopExtension = extensionsRegistry.get('welcome.banner');
+ const WelcomeDataExtension = uiOverrideRegistry.get('welcome.data');
+ const WelcomeTableExtension = uiOverrideRegistry.get('welcome.table');
Review Comment:
Is it necessary to have two extension points here? From the Superset
perspective, this extension is a general purpose "replace the homepage with
anything" component, that Preset happens to be using for some data and a table.
##########
superset-frontend/src/views/CRUD/welcome/Welcome.tsx:
##########
@@ -283,70 +285,80 @@ function Welcome({ user, addDangerToast }: WelcomeProps) {
return (
<WelcomeContainer>
{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 user={user} />}
+ {WelcomeTableExtension && (
+ <WelcomeTableExtension examples={activityData?.Examples} user={user} />
+ )}
Review Comment:
Oh, I see these are passed different data. If it's possible to combine these
and fetch data without the use of parameters, I think that would be preferable.
(in line with keeping extensions general purpose, not just tailored to what
Preset is using them for).
--
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]
