Skip to content

ETT-1141: role switch modal should appear after login #140

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

Merged
carylwyatt merged 9 commits into from
Jan 28, 2026
Merged

ETT-1141: role switch modal should appear after login #140

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6a36be6
add cookie logic for role switch prompt
carylwyatt Dec 17, 2025
8d487fc
add mock hasItem for tests, fix stories
carylwyatt Dec 17, 2025
3db9330
add navbar stories for overlapping modals
carylwyatt Jan 6, 2026
4497b1a
add component event for role switch modal
carylwyatt Jan 9, 2026
bd76287
update role switch modal
carylwyatt Jan 12, 2026
e296e41
fix failing tests
carylwyatt Jan 14, 2026
cb3c829
add same component prop to Modal to handle dismissing role switch modal
carylwyatt Jan 15, 2026
b6c9502
fix failing tests
carylwyatt Jan 15, 2026
6d6f0b6
update role prompt cookie expiration
carylwyatt Jan 26, 2026
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
77 changes: 40 additions & 37 deletions src/js/components/AdvancedSearchForm/AdvancedSearchForm.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,11 +1,16 @@
import AdvancedSearchForm from './index.svelte';
import AdvancedSearchDecorator from '../../decorators/AdvancedSearchDecorator.svelte'
import AdvancedSearchDecorator from '../../decorators/AdvancedSearchDecorator.svelte';
import { fn, expect, waitFor } from 'storybook/test';

export default {
title: 'AdvancedSearchForm',
component: AdvancedSearchForm,
decorators: [() => AdvancedSearchDecorator],
decorators: [
Copy link
Member Author

@carylwyatt carylwyatt Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

Most of the changes in this file are linting, but this is the important change here. Changing how this decorator is instantiated is what fixed the mysterious failing tests.

() => ({
Component: AdvancedSearchDecorator,
props: {},
}),
],
args: {
mockSubmit: fn(),
},
Expand All @@ -28,17 +33,15 @@ export const DefaultDesktop = {
},
},
play: async ({ args, canvas, userEvent }) => {

await canvas.getByLabelText('Search Term 1').focus();
await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant');
await userEvent.click(canvas.getByTestId('advanced-search-submit'));

await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled())
await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled());

const calledUrl = args.mockSubmit.mock.calls[0][0];
expect(calledUrl).toContain('q1=elephant')

}
expect(calledUrl).toContain('q1=elephant');
},
};

export const TwoFieldsWithAnd = {
Expand All @@ -49,7 +52,6 @@ export const TwoFieldsWithAnd = {
},
},
play: async ({ args, canvas, userEvent }) => {

await canvas.getByLabelText('Search Term 1').focus();
await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant');
const searchField2 = canvas.getByLabelText('Selected field 2');
Expand All @@ -58,13 +60,14 @@ export const TwoFieldsWithAnd = {
await userEvent.type(canvas.getByLabelText('Search Term 2'), 'conservation');
await userEvent.click(canvas.getByTestId('advanced-search-submit'));

await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled())
await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled());

const calledUrl = args.mockSubmit.mock.calls[0][0];
expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=AND')

}
}
expect(calledUrl).toContain(
'lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=AND'
);
},
};
export const FullTextFields1And4WithLastOr = {
globals: {
viewport: {
Expand All @@ -73,7 +76,6 @@ export const FullTextFields1And4WithLastOr = {
},
},
play: async ({ args, canvas, userEvent }) => {

await canvas.getByLabelText('Search Term 1').focus();
await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant');
const searchField4 = canvas.getByLabelText('Selected field 4');
Expand All @@ -84,13 +86,14 @@ export const FullTextFields1And4WithLastOr = {
await userEvent.click(thirdRadioGroupOr);
await userEvent.click(canvas.getByTestId('advanced-search-submit'));

await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled())
await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled());

const calledUrl = args.mockSubmit.mock.calls[0][0];
expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR')

}
}
expect(calledUrl).toContain(
'lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR'
);
},
};

export const FullTextFields1And4WithFirstOr = {
globals: {
Expand All @@ -100,7 +103,6 @@ export const FullTextFields1And4WithFirstOr = {
},
},
play: async ({ args, canvas, userEvent }) => {

await canvas.getByLabelText('Search Term 1').focus();
await userEvent.type(canvas.getByLabelText('Search Term 1'), 'elephant');
const firstRadioGroupOr = canvas.getAllByRole('radio', { name: 'OR' })[0];
Expand All @@ -111,13 +113,14 @@ export const FullTextFields1And4WithFirstOr = {
await userEvent.type(canvas.getByLabelText('Search Term 4'), 'conservation');
await userEvent.click(canvas.getByTestId('advanced-search-submit'));

await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled())
await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled());

const calledUrl = args.mockSubmit.mock.calls[0][0];
expect(calledUrl).toContain('lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR')

}
}
expect(calledUrl).toContain(
'lmt=ft&a=srchls&adv=1&q1=elephant&q2=conservation&field1=ocr&field2=subject&anyall1=all&anyall2=all&op2=OR'
);
},
};
export const CatalogFields1And4WithLastOr = {
globals: {
viewport: {
Expand All @@ -126,7 +129,6 @@ export const CatalogFields1And4WithLastOr = {
},
},
play: async ({ args, canvas, userEvent }) => {

const searchField1 = canvas.getByLabelText('Selected field 1');
await userEvent.selectOptions(searchField1, 'Title');
await canvas.getByLabelText('Search Term 1').focus();
Expand All @@ -139,13 +141,14 @@ export const CatalogFields1And4WithLastOr = {
await userEvent.click(thirdRadioGroupOr);
await userEvent.click(canvas.getByTestId('advanced-search-submit'));

await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled())
await waitFor(() => expect(args.mockSubmit).toHaveBeenCalled());

const calledUrl = args.mockSubmit.mock.calls[0][0];
expect(calledUrl).toContain('adv=1&setft=true&ft=ft&lookfor%5B%5D=apple&lookfor%5B%5D=orange&type%5B%5D=title&type%5B%5D=author&bool%5B%5D=OR')

}
}
expect(calledUrl).toContain(
'adv=1&setft=true&ft=ft&lookfor%5B%5D=apple&lookfor%5B%5D=orange&type%5B%5D=title&type%5B%5D=author&bool%5B%5D=OR'
);
},
};
export const InvalidDate = {
globals: {
viewport: {
Expand All @@ -162,10 +165,10 @@ export const InvalidDate = {

await waitFor(() => {
expect(args.mockSubmit).not.toHaveBeenCalled();
expect( canvas.getByText( /Publication Year must/)).toBeVisible();
expect(canvas.getByText(/Publication Year must/)).toBeVisible();
});
}
}
},
};

export const MissingSearch = {
globals: {
Expand All @@ -179,7 +182,7 @@ export const MissingSearch = {

await waitFor(() => {
expect(args.mockSubmit).not.toHaveBeenCalled();
expect( canvas.getByText( /A search term is required/)).toBeVisible();
expect(canvas.getByText(/A search term is required/)).toBeVisible();
});
}
}
},
};
14 changes: 10 additions & 4 deletions src/js/components/Modal/index.svelte
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import { onMount } from 'svelte';

import dialogPolyfill from 'dialog-polyfill';
import docCookies from '../../lib/cookies.svelte';

/**
* @typedef {Object} Props
Expand All @@ -12,6 +13,7 @@
* @property {boolean} [scrollable]
* @property {string} [mode]
* @property {boolean} [modalLarge]
* @property {boolean} [setPromptCookie]
* @property {boolean} [fullscreenOnMobile]
* @property {boolean} [focusHelpOnClose]
* @property {boolean} [focusMyAccountOnClose]
Expand All @@ -36,6 +38,8 @@
focusMyAccountOnClose = false,
focusButtonOnClose = false,
focusDownloadOnClose = false,
setPromptCookie = false,
Copy link
Member Author

@carylwyatt carylwyatt Jan 15, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

We don't normally want a modal to set this cookie, but if we pass the setPromptCookie prop in when we call the component, then we do want it to set the cookie.

checkForNotifications,
title,
body,
footer,
Expand Down Expand Up @@ -78,6 +82,9 @@
window.removeEventListener('keydown', logKeys);
console.log('-- dialog is closed');

if (setPromptCookie) {
docCookies.setItem('HT-role-prompt', 'true', 4, 'hours');
}
if (focusHelpOnClose) {
document.getElementById('get-help').focus();
}
Expand All @@ -97,10 +104,6 @@
console.log('-- polyfilling dialog');
dialogPolyfill.registerDialog(dialog);
}

if (isOpen) {
openModal();
}
});

$effect(() => {
Expand Down Expand Up @@ -131,6 +134,9 @@
data-bs-dismiss="modal"
onclick={() => {
hide();
if (setPromptCookie) {
Copy link
Member Author

@carylwyatt carylwyatt Jan 15, 2026

Choose a reason for hiding this comment

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

A roundabout way of telling the modal that we only want to check for notifications if this button is clicked from the RoleSwitchModal (the only modal component that passes in the setPromptCookie prop).

checkForNotifications()
}
}}
><span class="close-icon">
<i class="fa-solid fa-xmark icon-default" aria-hidden="true"></i><span class="fa-sr-only"
Expand Down
91 changes: 86 additions & 5 deletions src/js/components/Navbar/Navbar.stories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Navbar from './index.svelte';
import PingCallbackDecorator from '../../decorators/PingCallbackDecorator';
import { userEvent, within } from 'storybook/test';
import { expect } from 'storybook/test';
import { userEvent, within, waitFor, expect } from 'storybook/test';
// import { expect } from 'storybook/test';

export default {
title: 'Navbar',
Expand All @@ -24,7 +24,7 @@ export const Default = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);
//sanity check
expect(await canvas.getByTitle('HathiTrust Home')).toBeInTheDocument();
expect(canvas.getByTitle('HathiTrust Home')).toBeInTheDocument();
},
globals: {
viewport: {
Expand All @@ -44,7 +44,7 @@ export const DesktopDropdownMenuSelected = {
play: async ({ canvasElement }) => {
const canvas = within(canvasElement);

const mainMenu = await canvas.getByText(/member libraries/i);
const mainMenu = canvas.getByText(/member libraries/i);
await userEvent.click(mainMenu);
},
};
Expand Down Expand Up @@ -99,6 +99,25 @@ export const DesktopLoggedInResourceSharingRole = {
await userEvent.click(accountButton);
},
};
export const DesktopLoggedInResourceSharingRolePromptDismissed = {
parameters: { ...Default.parameters },
args: {
loggedIn: true,
},
decorators: [
() => ({
Component: PingCallbackDecorator,
props: { loggedIn: true, role: 'resourceSharing', hasActivatedRole: false },
}),
],
play: async ({ canvas }) => {
const closeModal = canvas.getByRole('button', { name: 'Cancel' });
await userEvent.click(closeModal);

const accountButton = canvas.getByLabelText(/My Account/);
await userEvent.click(accountButton);
},
};
export const DesktopLoggedInResourceSharingRoleActivated = {
parameters: { ...Default.parameters },
args: {
Expand All @@ -108,7 +127,14 @@ export const DesktopLoggedInResourceSharingRoleActivated = {
decorators: [
() => ({
Component: PingCallbackDecorator,
props: { loggedIn: true, role: 'resourceSharing', hasActivatedRole: true },
props: {
loggedIn: true,
role: 'resourceSharing',
hasActivatedRole: true,
cookieData: {
Copy link
Member Author

@carylwyatt carylwyatt Jan 15, 2026

Choose a reason for hiding this comment

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

Because we're mocking the cookie, the role switch modal does not appear during this story.

'HT-role-prompt': 'true',
},
},
}),
],
play: async ({ canvasElement }) => {
Expand Down Expand Up @@ -140,6 +166,61 @@ export const DesktopLoggedInWithNotifications = {
}),
],
};
export const DesktopLoggedInResourceSharingRoleAndNotification = {
parameters: { ...Default.parameters },
args: {
loggedIn: true,
hasNotification: true,
},
decorators: [
() => ({
Component: PingCallbackDecorator,
props: {
loggedIn: true,
role: 'resourceSharing',
hasActivatedRole: false,
cookieData: {},
notificationData: [{ title: 'What happens with two modals?', message: 'Hopefully these are not overlapping!' }],
},
}),
],
play: async ({ canvas, userEvent }) => {
const rolePromptCancelButton = canvas.getByRole('button', { name: 'Cancel' });
await userEvent.click(rolePromptCancelButton);
},
};
export const DesktopLoggedInRoleSwitchClosed = {
parameters: { ...Default.parameters },
args: {
loggedIn: true,
hasNotification: true,
},
decorators: [
() => ({
Component: PingCallbackDecorator,
props: {
loggedIn: true,
role: 'resourceSharing',
hasActivatedRole: false,
cookieData: {},
notificationData: [
{
title: 'Closed role switch modal',
message:
'User closed role switch modal using the X icon. Cookie was set, notifications should still appear.',
},
],
},
}),
],
play: async ({ canvas, userEvent }) => {
await waitFor(() => {
expect(canvas.getByRole('heading', { name: 'Choose a role' })).toBeVisible();
});
const rolePromptCloseButton = canvas.getByRole('button', { name: 'Close modal' });
await userEvent.click(rolePromptCloseButton);
},
};
export const Mobile = {
decorators: [
() => ({
Expand Down
Loading