Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

UIQM-728 Keep focus on last focused element when user cancels on confirmation modals. #767

Merged
merged 3 commits into from
Dec 16, 2024
Merged

UIQM-728 Keep focus on last focused element when user cancels on confirmation modals. #767

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
5b04a93
UIQM-728 Keep focus on last focused element when user cancels on conf…
BogdanDenis Dec 10, 2024
8befe59
UIQM-728 Remove re-focus last focused element logic for unlink fields…
BogdanDenis Dec 10, 2024
e81c774
Merge branch 'master' into UIQM-728
BogdanDenis Dec 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
* [UIQM-716](https://issues.folio.org/browse/UIQM-716) *BREAKING* Consolidate routes based on MARC type for bib and authority records to avoid page refresh after redirecting from the create page to the edit one.
* [UIQM-730](https://issues.folio.org/browse/UIQM-730) Create/Edit/Derive MARC record - Retain focus when MARC record validation rules error display. Show validation issues toasts.
* [UIQM-740](https://issues.folio.org/browse/UIQM-740) Don't show warn/fail error toasts when there are no warns/fails.
* [UIQM-728](https://issues.folio.org/browse/UIQM-728) Keep focus on last focused element when user cancels on confirmation modals.

## [9.0.2] (IN PROGRESS)

Expand Down
13 changes: 11 additions & 2 deletions src/QuickMarcEditor/QuickMarcEditor.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -447,6 +447,9 @@ const QuickMarcEditor = ({

const cancelUpdateLinks = () => {
setIsUpdate0101xxfieldsAuthRecModalOpen(false);
setTimeout(() => {
focusLastFocusedInput();
});
};

const confirmUpdateLinks = async (e) => {
Expand All @@ -461,17 +464,23 @@ const QuickMarcEditor = ({
};

const cancelDeleteFields = () => {
setIsDeleteModalOpened(false);

if (deletedRecords.length) {
restoreDeletedRecords();
} else {
reset();
}

setIsDeleteModalOpened(false);
setTimeout(() => {
focusLastFocusedInput();
});
};

const cancelRemoveLinking = () => {
setIsUnlinkRecordsModalOpen(false);
setTimeout(() => {
focusLastFocusedInput();
});
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  1. It looks like there is a need to close a modal first and then invoke the focusLastFocusedInput. Is it possible?
  2. Where should the focus be after a field is linked/unlinked?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I guess remove linking is not applicable for current story if it only happens instantly when a record has been opened for deriving. I'll remove this code because it's not needed.
But generally a modal will close first, and a last focused element will be re-focused second:

chrome_88Sf3gA0LK.mp4

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

But generally a modal will close first, and a last focused element will be re-focused second:

I meant setTimeout is used to trigger the focus after modal is closed (state is false) and whether it is possible to do it without setTimeout, just wait until modal state changes to false, but I think it will require a lot more changes.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Dmytro-Melnyshyn oh I see. it's not because we're waiting for the modal to close, but we need the form to finish all updates. If we set focus right away it'll be gone when the form updates. And there's no way to know when it does so we have to use setTimeout here

};

const confirmRemoveLinking = () => {
Expand Down
23 changes: 20 additions & 3 deletions src/QuickMarcEditor/QuickMarcEditor.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ const initialValues = {
indicators: ['2', '\\'],
id: 'test-id-1',
},
{
tag: '999',
content: '',
indicators: ['f', 'f'],
id: '999',
},
],
};

Expand Down Expand Up @@ -764,7 +770,7 @@ describe('Given QuickMarcEditor', () => {
});

describe('when click Cancel', () => {
it('should hide ConfirmationModal and restore deleted fields', async () => {
beforeEach(async () => {
const {
getAllByRole,
getByText,
Expand All @@ -780,10 +786,21 @@ describe('Given QuickMarcEditor', () => {

await act(async () => fireEvent.click(getByText('stripes-acq-components.FormFooter.save')));
await fireEvent.click(getByText('Cancel'));
});

it('should hide ConfirmationModal and restore deleted fields', async () => {
await waitFor(() => {
expect(queryByText('Confirmation modal')).toBeNull();
expect(getByText('$a Test title')).toBeDefined();
expect(screen.queryByText('Confirmation modal')).toBeNull();
expect(screen.getByText('$a Test title')).toBeDefined();
});
});

it('should keep focus on the last focused element', async () => {
const moveUpButtons = screen.getAllByRole('button', { name: 'ui-quick-marc.record.moveUpRow' });

await waitFor(() => {
expect(screen.getByText('$a Test title')).toBeDefined();
expect(moveUpButtons[moveUpButtons.length - 1]).toHaveFocus();
});
});
});
Expand Down
Loading