rusackas commented on code in PR #37625:
URL: https://github.com/apache/superset/pull/37625#discussion_r2770114682
##########
superset-frontend/src/dashboard/components/gridComponents/Tab/Tab.tsx:
##########
@@ -116,23 +123,55 @@ const TitleDropIndicator = styled.div`
}
`;
-const renderDraggableContent = dropProps =>
- dropProps.dropIndicatorProps && <div {...dropProps.dropIndicatorProps} />;
-
-const Tab = props => {
+interface DropIndicatorChildProps {
+ dropIndicatorProps?: {
+ className: string;
+ } | null;
+}
+
+const renderDraggableContent = (
+ dropProps: DropIndicatorChildProps,
+): ReactElement | null =>
+ dropProps.dropIndicatorProps ? (
+ <div {...dropProps.dropIndicatorProps} />
+ ) : null;
+
+interface DragDropChildProps {
+ dropIndicatorProps?: {
+ className: string;
+ } | null;
+ dragSourceRef?: Ref<HTMLDivElement>;
+ draggingTabOnTab?: boolean;
+}
+
+const Tab = (props: TabProps): ReactElement => {
const dispatch = useDispatch();
- const canEdit = useSelector(state => state.dashboardInfo.dash_edit_perm);
- const dashboardLayout = useSelector(state => state.dashboardLayout.present);
+ const canEdit = useSelector(
+ (state: RootState) => state.dashboardInfo.dash_edit_perm,
+ );
+ const dashboardLayout = useSelector(
+ (state: RootState) => state.dashboardLayout.present,
+ );
const lastRefreshTime = useSelector(
- state => state.dashboardState.lastRefreshTime,
+ (state: RootState) =>
+ (
+ state.dashboardState as RootState['dashboardState'] & {
+ lastRefreshTime?: number;
+ }
+ ).lastRefreshTime,
);
const tabActivationTime = useSelector(
- state => state.dashboardState.tabActivationTimes?.[props.id] || 0,
+ (state: RootState) =>
+ (
+ state.dashboardState as RootState['dashboardState'] & {
+ tabActivationTimes?: Record<string, number>;
+ }
+ ).tabActivationTimes?.[props.id] || 0,
Review Comment:
Agreed — `tabActivationTimes` should be part of `DashboardState`. The inline
type extension was the minimal approach for migration since adding it to
`DashboardState` requires verifying that it's properly initialized in the
reducer, handled in all action cases, and doesn't break other selectors. That's
a targeted DashboardState typing improvement worth doing as a follow-up.
##########
superset-frontend/src/dashboard/components/Header/useHeaderActionsDropdownMenu.tsx:
##########
@@ -271,7 +273,10 @@ export const useHeaderActionsMenu = ({
// Only add divider if there are items after it
const hasItemsAfterDivider =
(!editMode && reportMenuItem) ||
- (editMode && !isEmpty(dashboardInfo?.metadata?.filter_scopes));
+ (editMode &&
+ !isEmpty(
+ (dashboardInfo?.metadata as Record<string, unknown>)?.filter_scopes,
+ ));
Review Comment:
Agreed — defining a `DashboardMetadata` interface with known properties like
`filter_scopes` would eliminate these repeated casts. This ties into the
broader `DashboardInfo` typing effort mentioned in the `dashboardInfo.ts`
comments below. Will include in the follow-up.
--
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]
