aminghadersohi commented on code in PR #45336:
URL: https://github.com/apache/superset/pull/45336#discussion_r4283091227
##########
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 cancelled main-button press cases. Fixed in
c652ec7d3188932e7d919d432ad066bec1e2c1b8: the shared hook expires a released
gesture after the release-triggered click can dispatch, clears cancelled
pointers and keyboard gestures, and removes listeners/timers on unmount. SQL
Lab forwards blur from both the plain and CTAS/CVAS main buttons.
Regression tests (eight cases failed with the original hook at
5e8b24abbf5a3200aa80b0b1a22d89f8f487343b, then passed with
c652ec7d3188932e7d919d432ad066bec1e2c1b8; each previously expected one
intentional activation but received zero):
- Regression test:
`superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx::an
assistive technology click still runs after a cancelled %s main-button pointer
press` (plain and CTAS).
- Regression test:
`superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx::an
assistive technology click still runs after a Space press cancelled by %s`
(Escape, blur, keyup without a click).
- Regression test:
`superset-frontend/src/SqlLab/components/RunQueryActionButton/RunQueryActionButton.test.tsx::an
assistive technology click still runs after main-button %s` (pointerCancel,
lostPointerCapture).
- Regression test:
`superset-frontend/src/explore/components/RunQueryButton/RunQueryButton.test.tsx::an
assistive technology click still updates the chart after a cancelled pointer
press`.
The existing held-Enter, held-Space, pointer-spanning-completion and CTAS
caret tests also pass. `moving focus during a pointer press preserves the
starting Stop action` additionally checks that moving focus during an ongoing
pointer press does not erase its starting action.
Commands run from `superset-frontend`:
- `npm run test -- src/SqlLab/components/RunQueryActionButton
src/explore/components/RunQueryButton --maxWorkers=2`: original hook, 8 failed
/ 30 passed; after, 38 passed, exit 0.
- `npm run test -- src/SqlLab src/explore/components/RunQueryButton
--maxWorkers=2`: 47 suites passed, 490 tests passed, one skipped; exit 0 with a
Jest worker-shutdown warning.
- Repository-root `pre-commit run --files` with the six explicit
branch-changed paths: exit 0, including frontend type checking.
Siblings checked:
- superset-frontend/src/hooks/useRunStopButton.ts:24 — fixed in
c652ec7d3188932e7d919d432ad066bec1e2c1b8
- superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:129
— fixed in c652ec7d3188932e7d919d432ad066bec1e2c1b8
- superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:155
— fixed in c652ec7d3188932e7d919d432ad066bec1e2c1b8
- superset-frontend/src/explore/components/RunQueryButton/index.tsx:47 —
fixed in c652ec7d3188932e7d919d432ad066bec1e2c1b8
- superset-frontend/src/SqlLab/components/RunQueryActionButton/index.tsx:88
— not affected: CTAS/CVAS caret presses are excluded from main-button capture;
both existing caret key and pointer regression tests pass.
-
superset-frontend/packages/superset-ui-core/src/components/DropdownButton/index.tsx:63
— not affected: forwards the main action and opens the caret menu without
storing a gesture.
- superset-frontend/src/explore/components/ControlPanelsContainer.tsx:1135 —
not affected: delegates to RunQueryButton.
- superset-frontend/src/SqlLab/components/SqlEditor/index.tsx:337 — not
affected: explicit shortcut Run and Stop callbacks are separate from native
button gestures.
-
superset-frontend/src/explore/components/ExploreViewContainer/index.tsx:590 —
not affected: its Run shortcut and cancellation callback do not store a native
button gesture.
--
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]
