lizhimins commented on PR #6045:
URL:
https://github.com/apache/rocketmq-dashboard/pull/6045#issuecomment-6120330858
Thanks for the resubmission. I don't think this one is needed on the current
tip: `handleAck` already sets `acknowledgingIds` **synchronously before the
first `await`** (`web/src/pages/ops/systemAlerts.tsx:246`, `await
acknowledgeAlert(id)` is at `:248`), and the confirm button consumes it as
`loading={acknowledgingIds.has(alert.id)}` (`:835`). In a browser, two physical
clicks are two separate discrete-event tasks and React 18 flushes sync work at
the end of each one, so the second click lands on a button that is already in
`ant-btn-loading`, which antd v5 swallows (`Button.handleClick` returns early
when `innerLoading`). The ref only helps for same-tick re-entry, and
`handleAck` has exactly one call site (`:834`) with no loop / bulk / keyboard
path that could produce it. Related: the new test uses `userEvent.dblClick`,
which gives React a chance to flush between the two clicks, so it also passes
on the unpatched tip and therefore does not pin the ref. Contrast with the
audit cle
anup modal (#6044), where the trunk genuinely has no guard at all — that one
is a real fix. If you want to keep this as belt-and-braces hardening, please
make the test click twice inside a single tick (`fireEvent.click(btn);
fireEvent.click(btn);`) so removing the ref would actually turn it red;
otherwise I'd suggest closing this in favour of a change that covers a path the
state guard does not (e.g. the silence/delivery retry buttons on the same page).
--
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]
