EnxDev commented on code in PR #43296:
URL: https://github.com/apache/superset/pull/43296#discussion_r3846693950
##########
superset-frontend/src/explore/components/DataTablesPane/components/DataTableControls.tsx:
##########
@@ -111,14 +112,18 @@ export const TableControls = ({
value={rowLimit}
onChange={onRowLimitChange}
options={rowLimitOptions ?? []}
+ // Labelled as the applied limit to avoid a second row count next
to RowCountLabel.
+ prefix={t('Limit')}
css={css`
- min-width: 110px;
+ min-width: 160px;
`}
/>
)}
- {(!onRowLimitChange || rowcount < (rowLimit ?? Infinity)) && (
- <RowCountLabel rowcount={rowcount} loading={isLoading} />
- )}
+ <RowCountLabel
+ rowcount={rowcount}
+ limit={effectiveRowLimit ?? rowLimit}
Review Comment:
No. SamplesPane's row limit is just its own page-size selector (e.g.
DEFAULT_ROW_LIMIT = 100), not a chart's row_limit — there's no "chart" involved
in Samples at all. Showing "The row limit set for the chart was reached. The
chart may show partial data." there was a genuine copy bug. Fix: added an
optional limitReachedMessage prop on RowCountLabel (and threaded through
TableControlsProps) that lets a caller override the default chart wording.
SamplesPane now passes its own message instead: "The sample row limit was
reached. This dataset may contain more rows." SingleQueryResultPane (the real
chart-results pane) doesn't pass it, so it keeps the original chart-specific
copy, which is correct there.
--
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]