RoySerbi commented on PR #40563: URL: https://github.com/apache/superset/pull/40563#issuecomment-4599311796
@aminghadersohi thanks for the thorough review. `1a7e4dd` addresses each item: **BLOCKER — rename test rewritten** The `should dispatch queryEditorSetTitle action` test no longer stubs `window.prompt`. It now drives the full modal flow: clicks `rename-tab-menu-option`, waits for `rename-tab-input` to appear, fires `change` with the desired title, clicks `rename-tab-confirm`, and asserts `QUERY_EDITOR_SET_TITLE` is dispatched. Renamed to `should dispatch queryEditorSetTitle action via modal confirm`. Also added the two follow-up cases you suggested: - `should dispatch queryEditorSetTitle action when pressing Enter` - fires a keydown Enter on the input and verifies the same dispatch (covers onPressEnter). - `cancel button closes modal without dispatching` - types text, clicks Cancel, asserts the modal unmounts and no QUERY_EDITOR_SET_TITLE was dispatched. Plus an extra: - `confirm with empty input does not dispatch and keeps modal open` - covers the codeant-bot edge case where pressing Enter on empty/whitespace input must not silently close the modal. The existing early-return at `index.tsx:120` already handles this; the new test pins the behaviour. The Cancel test required exposing `data-test="rename-tab-cancel"` on the Cancel button - included in this commit. **MEDIUM - autoFocus added** Added `autoFocus` to the rename `<Input>`. The input is now focused as soon as the modal opens. **NIT - trim on change** `onChange` now sets `e.target.value.trim()`, so the displayed value matches what would be saved. (Quick note: String.trim() only strips leading/trailing whitespace, so internal spaces are preserved. One edge case is typing a trailing space and then more text strips the space; happy to switch to a leading-only strip + onBlur trim if you prefer.) -- 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]
