reynoldmorel commented on code in PR #37112:
URL: https://github.com/apache/superset/pull/37112#discussion_r2700560283
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -186,14 +186,24 @@ const SliceHeader = forwardRef<HTMLDivElement,
SliceHeaderProps>(
({ dashboardInfo }) => dashboardInfo.crossFiltersEnabled,
);
- const firstQueryResponse = useSelector<RootState, QueryData | undefined>(
- state => state.charts[slice.slice_id].queriesResponse?.[0],
+ const queriesResponse = useSelector<RootState, QueryData[] | null |
undefined>(
+ state => state.charts[slice.slice_id].queriesResponse,
);
const theme = useTheme();
- const rowLimit = Number(formData.row_limit || -1);
- const sqlRowCount = Number(firstQueryResponse?.sql_rowcount || 0);
+ const rowLimit = Number(formData.row_limit ?? 0);
Review Comment:
Why are we changing this from `-1` to `0`, are you sure this is not breaking
any existing logic?
I agree with this change though, if it is safe xD!
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -304,18 +314,20 @@ const SliceHeader = forwardRef<HTMLDivElement,
SliceHeaderProps>(
<FiltersBadge chartId={slice.slice_id} />
)}
- {shouldShowRowLimitWarning && sqlRowCount === rowLimit && (
+ {!uiConfig.hideChartControls && isTableChart && sqlRowCount > 0
&& (
<RowCountLabel
rowcount={sqlRowCount}
limit={rowLimit}
label={
- <Icons.WarningOutlined
- iconSize="l"
- iconColor={theme.colorWarning}
- css={theme => css`
- padding: ${theme.sizeUnit}px;
- `}
- />
+ shouldShowRowLimitWarning && sqlRowCount >= rowLimit &&
rowLimit > 0 ? (
Review Comment:
nit: Also make these flags variables to understand a bit faster what they
are doing. For example:
```
const showWarningMsg = showRowCountLabel && sqlRowCount >= rowLimit &&
rowLimit > 0;
```
##########
superset-frontend/src/dashboard/components/SliceHeader/index.tsx:
##########
@@ -304,18 +314,20 @@ const SliceHeader = forwardRef<HTMLDivElement,
SliceHeaderProps>(
<FiltersBadge chartId={slice.slice_id} />
)}
- {shouldShowRowLimitWarning && sqlRowCount === rowLimit && (
+ {!uiConfig.hideChartControls && isTableChart && sqlRowCount > 0
&& (
Review Comment:
nit: Also make these flags variables to understand a bit faster what they
are doing. For example:
```
const showRowCountLabel = !uiConfig.hideChartControls && isTableChart &&
sqlRowCount > 0;
```
--
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]
