fisjac commented on code in PR #30958:
URL: https://github.com/apache/superset/pull/30958#discussion_r1847118166
##########
superset-frontend/src/dashboard/components/SyncDashboardState/index.tsx:
##########
@@ -55,36 +52,43 @@ const updateDashboardTabLocalStorage = (
const dashboardsContexts = getDashboardContextLocalStorage();
setItem(LocalStorageKeys.DashboardExploreContext, {
...dashboardsContexts,
- [dashboardPageId]: dashboardContext,
+ [dashboardPageId]: { ...dashboardContext, dashboardPageId },
});
};
+const selectDashboardContextForExplore = createSelector(
+ [
+ (state: RootState) => state.dashboardInfo.metadata,
+ (state: RootState) => state.dashboardInfo.id,
+ (state: RootState) => state.dashboardState?.colorScheme,
+ (state: RootState) => state.nativeFilters?.filters,
+ (state: RootState) => state.dataMask,
+ ],
+ (metadata, dashboardId, colorScheme, filters, dataMask) => {
+ // Building nativeFilters object without spreading in reduce
+ const optimizedNativeFilters = Object.keys(filters).reduce((acc, key) => {
Review Comment:
Nice improvement. Nit: should this be named `nativeFilters` outside of the
context of this PR, I don't think "optimized" make sense.
##########
superset-frontend/src/dashboard/components/DashboardBuilder/DashboardBuilder.tsx:
##########
@@ -593,33 +634,7 @@ const DashboardBuilder: FC<DashboardBuilderProps> = () => {
maxWidth={OPEN_FILTER_BAR_MAX_WIDTH}
initialWidth={OPEN_FILTER_BAR_WIDTH}
>
- {adjustedWidth => {
- const filterBarWidth = dashboardFiltersOpen
- ? adjustedWidth
- : CLOSED_FILTER_BAR_WIDTH;
- return (
- <FiltersPanel
- width={filterBarWidth}
- hidden={isReport}
- data-test="dashboard-filters-panel"
- >
- <StickyPanel ref={containerRef} width={filterBarWidth}>
- <ErrorBoundary>
- <FilterBar
- orientation={FilterBarOrientation.Vertical}
- verticalConfig={{
- filtersOpen: dashboardFiltersOpen,
- toggleFiltersBar: toggleDashboardFiltersOpen,
- width: filterBarWidth,
- height: filterBarHeight,
- offset: filterBarOffset,
- }}
- />
- </ErrorBoundary>
- </StickyPanel>
- </FiltersPanel>
- );
- }}
+ {renderChild}
Review Comment:
Nice!
##########
superset-frontend/src/dashboard/components/DashboardBuilder/state.ts:
##########
@@ -43,24 +43,28 @@ export const useNativeFilters = () => {
const nativeFiltersEnabled =
canEdit || (!canEdit && filterValues.length !== 0);
- const requiredFirstFilter = filterValues.filter(
- filter => filter.requiredFirst,
+ const requiredFirstFilter = useMemo(
+ () => filterValues.filter(filter => filter.requiredFirst),
+ [filterValues],
);
const dataMask = useNativeFiltersDataMask();
- const missingInitialFilters = requiredFirstFilter
- .filter(({ id }) => dataMask[id]?.filterState?.value === undefined)
- .map(({ name }) => name);
+ const missingInitialFilters = useMemo(
+ () =>
+ requiredFirstFilter
+ .filter(({ id }) => dataMask[id]?.filterState?.value === undefined)
+ .map(({ name }) => name),
+ [requiredFirstFilter, dataMask],
+ );
+
const showDashboard =
isInitialized ||
!nativeFiltersEnabled ||
missingInitialFilters.length === 0;
- const toggleDashboardFiltersOpen = useCallback(
- (visible?: boolean) => {
- setDashboardFiltersOpen(visible ?? !dashboardFiltersOpen);
- },
- [dashboardFiltersOpen],
- );
+
+ const toggleDashboardFiltersOpen = useCallback((visible?: boolean) => {
+ setDashboardFiltersOpen(prevState => visible ?? !prevState);
+ }, []);
Review Comment:
nice!
--
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]
