mikebridge commented on PR #43836: URL: https://github.com/apache/superset/pull/43836#issuecomment-5543642141
@aminghadersohi — you were right, and thanks for actually running the `isPanelOpen: true` control: the closed-state test was vacuous. Both mocks conspired — the `VersionHistoryPanel` mock rendered `null` (the larger half, as you noted) and `restoreModal` was mocked to `null` (bito's half) — so the container was empty in *both* states and `toBeEmptyDOMElement` could never fail. Fixed in the new head: the panel mock now renders a marker element, and the restore-modal mock portals a marker into `document.body` (mirroring the real Modal, which is exactly why the contract is about *in-place* emptiness). The closed-state test now asserts: container empty **and** the modal marker alive outside the column **and** no panel marker anywhere; a companion open-state test asserts the same container is non-empty with the panel marker present. Verified by both controls: flipping `isPanelOpen` to `true` fails exactly the closed-state test, and a production regression (wrapping the closed return in a `<div>`) fails it too — the `:empty` shadow guard's contract is genuinely load-bearing on this suite now. (The media-query branch itself remains outside jsdom's reach; the Playwright activity-log spec at 1280px exercises the overlay path in CI, per the earlier review note.) Your Escape-to-close a11y nudge and the chat-panel/Splitter-below-XXL edge are noted as follow-ups rather than folded in here. -- 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]
