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.
✅ **Addressed in commit 7265f63d4a** - Added `lastRefreshTime` and
`tabActivationTimes` to `DashboardState` type, removed inline type extensions
from Tab.tsx selectors.
##########
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.
✅ **Addressed in commit 7265f63d4a** - Added `filter_scopes` to
`DashboardInfo.metadata` type, removed the `Record<string, unknown>` cast.
--
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]
