aminghadersohi commented on code in PR #45336:
URL: https://github.com/apache/superset/pull/45336#discussion_r4282962851
##########
superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:
##########
@@ -109,6 +118,32 @@ const RunQueryActionButton = ({
const shouldShowStopBtn =
!!queryState && ['running', 'pending'].indexOf(queryState) > -1;
+ const currentAction: ButtonAction = shouldShowStopBtn ? 'stop' : 'run';
+
+ // The label can switch between Stop and Run while a pointer or key is held
+ // down, and the browser dispatches the click against whatever the button
+ // shows at release time. Remember the action the gesture started on so a
+ // press that began on Stop can never run the query.
+ const gestureActionRef = useRef<ButtonAction | null>(null);
+
+ const captureGestureAction = () => {
+ gestureActionRef.current = currentAction;
+ };
+
+ const onKeyDownCapture = (event: KeyboardEvent<HTMLSpanElement>) => {
+ if (!event.repeat && (event.key === 'Enter' || event.key === ' ')) {
+ captureGestureAction();
+ }
Review Comment:
I reproduced the Explore sibling and the CTAS/CVAS caret stale gesture.
Fixed in 5e8b24abbf5a3200aa80b0b1a22d89f8f487343b: Explore and SQL Lab share
the gesture guard, and SQL Lab records gestures only on the main action button,
not the caret.
Regression tests added before the implementation change, all failing at
b45e88f86fe85b71910329bd233ad2f085bce076 and passing afterward:
- Regression test:
`superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx::held
Enter on Stop does not update the chart after completion` — before: two
unwanted chart queries; after: zero, with a fresh Enter press still executing
once.
- Regression test:
`superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx::a
pointer gesture on Stop does not update the chart after completion` — before:
one unwanted chart query; after: zero, with a fresh pointer gesture still
executing once.
- Regression test:
`superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx::held
Space on Stop does not update the chart after completion` — before: one
unwanted chart query; after: zero.
- Regression test:
`superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx::an
assistive technology click still runs after a CTAS caret key press` — before:
the no-keydown/no-pointerdown main-button click was swallowed; after: executes
once.
- Regression test:
`superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx::an
assistive technology click still runs after a CTAS caret pointer press` — same
failure and passing result for the pointer sibling.
`npm run test -- src/SqlLab/components/RunQueryActionButton
src/explore/components/RunQueryButton --maxWorkers=2`: before, 5 failed / 24
passed; after, 29 passed. `npm run test -- src/SqlLab
src/explore/components/RunQueryButton --maxWorkers=2`: 47 suites passed, 481
tests passed, one skipped (exit 0; Jest also emitted a worker-shutdown
warning). `pre-commit run` on the six explicitly staged paths: exit 0,
including frontend type checking.
Siblings checked:
- superset-frontend/src/explore/components/RunQueryButton/index.tsx:47 —
fixed in 5e8b24abbf5a3200aa80b0b1a22d89f8f487343b
- superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:129
— fixed in 5e8b24abbf5a3200aa80b0b1a22d89f8f487343b
- superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:157
— fixed in 5e8b24abbf5a3200aa80b0b1a22d89f8f487343b
- superset-frontend/src/hooks/useRunStopButton.ts:24 — fixed in
5e8b24abbf5a3200aa80b0b1a22d89f8f487343b
- superset-frontend/src/explore/components/ControlPanelsContainer.tsx:1135 —
not affected: delegates both actions to the guarded RunQueryButton rather than
rendering another dual-action Button.
-
superset-frontend/src/explore/components/ExploreViewContainer/index.tsx:590 —
not affected: the keyboard shortcut explicitly invokes Run and cancellation
only aborts its controller; neither swaps a native button action.
- superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:337 — not
affected: its separate keyboard Run and Stop handlers do not swap button
actions; CTAS/CVAS menu items open a modal and delegate the toolbar control to
RunQueryActionButton.
-
superset-frontend/src/SqlLab/components/KeyboardShortcutButton/index.tsx:46 —
not affected: displays shortcut descriptions, not a Run/Stop action button.
- superset-frontend/src/dashboard/hooks/useAutoRefreshTabPause.ts:65 — not
affected: stops a refresh timer on visibility changes, not a query button
gesture.
-
superset-frontend/src/dashboard/components/Header/useHeaderAutoRefresh.ts:404 —
not affected: timer callbacks are not a native Run/Stop button.
- superset-frontend/src/core/sqlLab/index.ts:664 — not affected: extension
cancellation dispatches only Stop operations and has no native button gesture.
I searched across all of `superset-frontend/src` with `rg -n
'onStop|stopQuery|[\x27\"]Stop[\x27\"]' superset-frontend/src --glob
'!*.test.*' --glob '!*.stories.*'` and a broader case-insensitive search for
stop/query and stop-button labels. The two dual-action button components found
are listed above.
--
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]
