Copilot commented on code in PR #40832:
URL: https://github.com/apache/superset/pull/40832#discussion_r3383171320
##########
superset-frontend/src/dashboard/components/Header/Header.test.tsx:
##########
@@ -522,9 +522,9 @@ test('should disable both buttons when no actions
available', () => {
expect(onRedo).not.toHaveBeenCalled();
});
-test('should render the "Discard changes" button', () => {
+test('should render the "Discard" button as disabled', () => {
setup(editableState);
- expect(screen.getByText('Discard')).toBeInTheDocument();
+ expect(screen.getByText('Discard').parentElement).toBeDisabled();
});
Review Comment:
The assertion `screen.getByText('Discard').parentElement` is brittle because
it depends on the internal DOM structure of the Button component. Prefer
querying the actual button element via `getByRole('button', { name: ... })` so
the test remains stable if the markup changes (and it better reflects user
interactions).
##########
superset-frontend/src/dashboard/components/Header/index.tsx:
##########
@@ -721,6 +721,7 @@ const Header = (): JSX.Element => {
<Button
css={discardBtnStyle}
buttonSize="small"
+ disabled={!hasUnsavedChanges}
onClick={discardChanges}
buttonStyle="secondary"
data-test="discard-changes-button"
Review Comment:
Disabling the Discard button when `!hasUnsavedChanges` also removes the only
Header control that exits edit mode (this button’s handler just strips
`?edit`). In edit mode with no changes, users may be unable to return to view
mode from the toolbar without first making a change. Consider keeping an
always-enabled “Exit edit mode” action (or keeping Discard enabled when there
are no changes but adjusting its label/tooltip) so the user can leave edit mode
even when nothing is dirty.
--
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]
