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.

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

[NO-TICKET] Use Popover API in the Menu component #2628

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 7 additions & 1 deletion frontend/src/components/ActivityReportsTable/ReportRow.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,13 @@ function ReportRow({
<td data-label="Last saved">{lastSaved}</td>
<td data-label="Approved at">{approvedAt && moment(approvedAt).format(DATE_DISPLAY_FORMAT)}</td>
<td data-label="Context menu">
<ContextMenu label={contextMenuLabel} menuItems={menuItems} up={openMenuUp} fixed />
<ContextMenu
label={contextMenuLabel}
menuItems={menuItems}
up={openMenuUp}
menuWidthOffset={130}
fixed
/>
</td>
</tr>
);
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/GoalCards/GoalCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ export default function GoalCard({
</div>
{ !hideGoalOptions && (
<ContextMenu
fixed
label={contextMenuLabel}
menuItems={menuItems}
menuWidthOffset={100}
Expand Down
1 change: 1 addition & 0 deletions frontend/src/components/GoalForm/ReadOnlyGoal.js
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,7 @@ export default function ReadOnlyGoal({
<h2 className="margin-top-0 margin-bottom-3">Recipient TTA goal</h2>
<div className="position-absolute pin-top pin-right padding-4">
<ContextMenu
fixed
label={`Actions for Goal ${goal.id}`}
menuItems={menuItems}
menuClassName="width-card"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ export default function ReadOnlyOtherEntityObjectives({
!hideEdit
? (
<ContextMenu
fixed
label="Actions for Objectives"
menuItems={menuItems}
menuClassName="width-card"
Expand Down
68 changes: 32 additions & 36 deletions frontend/src/components/Menu.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
/* eslint-disable react/no-unknown-property */
/*
Context menu shows a list of actions the user can select to perform actions. The actions
are hidden until the user opens the menu (the three dot icon). The menu is automatically
Expand Down Expand Up @@ -31,6 +32,7 @@ function Menu({
const [menuPosition, updateMenuPosition] = useState({});
const defaultClass = 'smart-hub--menu';
const buttonRef = useRef(null);
const popoverRef = useRef(null);

const onEscape = useCallback((event) => {
if (event.keyCode === ESCAPE_KEY_CODE) {
Expand All @@ -47,44 +49,27 @@ function Menu({
}, [onEscape]);

const recordButtonPositionAndUpdateMenu = useCallback(() => {
// set initial postition
if (fixed && buttonRef.current && buttonRef.current.getBoundingClientRect) {
// get the button's position
const {
top,
height,
left: l,
width,
} = buttonRef.current.getBoundingClientRect();

// we could be progratically calculating the height and width offset numbers
// but a little manual work up front will save on performance in the browser

let leftPos = l + width;

// left = the menu opens to the left of the button instead of the right
if (left) {
leftPos = l + width - menuWidthOffset;
}
if (!(fixed && buttonRef.current && buttonRef.current.getBoundingClientRect)) {
return;
}

// top = the menu opens above the button instead of below
let topPos = top + height;
const {
x,
y,
height,
width,
} = buttonRef.current.getBoundingClientRect();

if (up) {
topPos = top - height - menuHeightOffset;
}
const leftPos = left ? x + width - menuWidthOffset : x + width;

// update the CSS
updateMenuPosition({ top: `${topPos}px`, left: `${leftPos}px` });
}
// top = the menu opens above the button instead of below
const topPos = up ? y - height - menuHeightOffset : y + height;

updateMenuPosition({ top: `${topPos}px`, left: `${leftPos}px`, margin: 0 });
}, [fixed, left, menuHeightOffset, menuWidthOffset, up]);

// watch for window scroll
useEffect(() => {
// the menu position is based on the button position, but because it is encased in a
// no-overflow div, we position it using "fixed"
// this means that it wouldn't scroll with the page, so we need to update the position
// when the user scrolls
if (fixed) {
window.addEventListener('scroll', recordButtonPositionAndUpdateMenu);
}
Expand All @@ -94,6 +79,12 @@ function Menu({
};
}, [fixed, recordButtonPositionAndUpdateMenu]);

useEffect(() => {
if (shown) {
recordButtonPositionAndUpdateMenu();
}
}, [recordButtonPositionAndUpdateMenu, shown]);

const onBlur = (e) => {
const { currentTarget } = e;

Expand Down Expand Up @@ -124,10 +115,6 @@ function Menu({
const menuClass = `${defaultClass} shadow-1 z-top ${positionClass} ${placementClass}`;

const onClick = () => {
// we don't need to check anything before calling this - if fixed is false, it won't do anything
recordButtonPositionAndUpdateMenu();

// toggle the menu visibility
updateShown((previous) => !previous);
};

Expand All @@ -144,11 +131,20 @@ function Menu({
type="button"
data-testid={buttonTestId}
ref={buttonRef}
popovertarget="menu-popover"
popovertargetaction="toggle"
>
{buttonText}
</button>
{shown && (
<div data-testid="menu" className={menuClass} style={{ backgroundColor, ...menuPosition }}>
<div
id="menu-popover"
ref={popoverRef}
popover="manual"
data-testid="menu"
className={menuClass}
style={{ backgroundColor, ...menuPosition }}
>
<ul className="usa-list usa-list--unstyled" role="menu">
{menuItems.map((item) => (
<li key={item.label} role="menuitem">
Expand Down
14 changes: 10 additions & 4 deletions frontend/src/components/Menu.scss
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
@use '../colors.scss' as *;

.usa-form .usa-button.smart-hub--menu-button,
.smart-hub--menu-button {
display: block;
text-decoration: none;
width: max-content;
height: 100%;
color: $text-ink;
margin: 0;
}

.smart-hub--menu-button:hover {
Expand All @@ -14,13 +16,17 @@
}

.position-absolute.smart-hub--menu__left_and_up {
transform: translateY(-120%) translateX(-90%);
transform: translateY(-120%) translateX(-90%);
}

.position-absolute.smart-hub--menu__left {
transform: translateX(-90%);
transform: translateX(-90%);
}

.position-absolute.smart-hub--menu__up {
transform: translateY(-120%);
}
transform: translateY(-120%);
}

[popover] {
border: 0;
}
2 changes: 2 additions & 0 deletions frontend/src/components/WidgetContainerTitleGroup.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,8 +63,10 @@ const WidgetContainerTitleGroup = ({
)}
{(menuItems.length > 0 && (
<ContextMenu
fixed
menuItems={menuItems}
label={`Open Actions for ${title}`}
menuWidthOffset={140}
/>
))}
</div>
Expand Down
1 change: 1 addition & 0 deletions frontend/src/pages/TrainingReports/components/EventCard.js
Original file line number Diff line number Diff line change
Expand Up @@ -253,6 +253,7 @@ function EventCard({
<div className="ttahub-event-card__event-column ttahub-event-card__event-column__menu position-absolute right-0">
{menuItems.length > 0 && (
<ContextMenu
fixed
label={contextMenuLabel}
menuItems={menuItems}
/>
Expand Down
2 changes: 1 addition & 1 deletion frontend/src/widgets/HorizontalTableWidget.js
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@ export default function HorizontalTableWidget(
fixed
label="Actions for Communication Log"
menuItems={r.actions}
menuWidthOffset={110}
menuWidthOffset={100}
/>
</td>
) : null}
Expand Down
40 changes: 20 additions & 20 deletions tests/e2e/activity-report.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ async function activitySummary(
if (recipients) {
// select recipients
for (let i = 0; i < recipients; i++) {
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');
}
} else {
await page.keyboard.press('Enter');
Expand Down Expand Up @@ -250,7 +250,7 @@ test.describe('Activity Report', () => {
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await blur(page);
await page. locator('[id="goalForEditing\.objectives\[0\]\.title"]').fill('g2o1');
await page. locator('[id="goalForEditing\.objectives\[0\]\.title"]').fill('g2o1');
await page.getByText('Topics *').click()
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
Expand All @@ -273,7 +273,7 @@ test.describe('Activity Report', () => {

// assert the goals and objectives section is complete
let sideNavTextContent = await page.locator('#activityReportSideNav-goals-and-objectives .page-state').textContent();

await page.waitForTimeout(10000);

expect(sideNavTextContent?.match(/Complete/i)).toBeTruthy();
Expand All @@ -297,7 +297,7 @@ test.describe('Activity Report', () => {
sideNavTextContent = await page.locator('#activityReportSideNav-goals-and-objectives .page-state').textContent();

expect(sideNavTextContent?.match(/in progress/i)).toBeTruthy();

// save the first goal
await page.getByRole('button', { name: 'Save goal' }).click();

Expand Down Expand Up @@ -513,7 +513,7 @@ test.describe('Activity Report', () => {
// enter goal name
await page.getByLabel('Recipient\'s goal *').fill('This is a goal for multiple grants');
await page.getByRole('button', { name: 'Save and continue' }).click();

await page.waitForTimeout(5000);

// select recipients
Expand Down Expand Up @@ -576,9 +576,9 @@ test.describe('Activity Report', () => {

await page.getByText('Topics *').click()
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');
await blur(page);

const supportType = page.getByRole('combobox', { name: /Support type/i });
await supportType.selectOption('Implementing');

Expand Down Expand Up @@ -650,12 +650,12 @@ test.describe('Activity Report', () => {
// create a new report
await page.getByRole('link', { name: 'Activity Reports' }).click();
await page.getByRole('button', { name: '+ New Activity Report' }).click();

const heading = page.getByRole('heading', { name: /activity report for region \d/i });
const regionNumber = await heading.textContent().then((text) => text!.match(/\d/)![0]);
const regionNumber = await heading.textContent().then((text) => text!.match(/\d/)![0]);

await activitySummary(page, { recipients: 2, ttaType: 'Training' });

// select two recipients
await page.locator('label').filter({ hasText: 'Other entity' }).click();
await page.locator('#activityRecipients div').filter({ hasText: '- Select -' }).nth(1).click();
Expand All @@ -666,7 +666,7 @@ test.describe('Activity Report', () => {
await page.getByRole('button', { name: 'Goals and objectives Not started' }).click();
await page.getByRole('button', { name: 'Supporting attachments Not started' }).click();
await page.getByRole('button', { name: 'Next steps Not started' }).click();
await page.getByRole('button', { name: 'Review and submit' }).click();
await page.getByRole('button', { name: 'Review and submit' }).click();
await page.getByRole('button', { name: 'Activity summary Complete' }).click();

// select participants
Expand Down Expand Up @@ -713,7 +713,7 @@ test.describe('Activity Report', () => {

// skip supporting attachments
await page.getByRole('button', { name: 'Save and continue' }).click();

// fill out next steps
const isOtherEntity = true;
await nextSteps(page, isOtherEntity);
Expand All @@ -723,7 +723,7 @@ test.describe('Activity Report', () => {
await page.getByLabel(/Approving manager/i).focus();
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');

// extract the AR number from the URL:
const url = page.url();
const arNumber = url.split('/').find((part) => /^\d+$/.test(part));
Expand Down Expand Up @@ -755,14 +755,14 @@ test.describe('Activity Report', () => {
await page.getByLabel('Topics *').press('ArrowDown');
await page.getByLabel('Topics *').press('Enter');
await page.getByLabel('Topics *').press('Tab');

await blur(page);

await page.getByText('TTA provided *Normal').click();
await page.getByRole('textbox', { name: 'TTA provided for objective, required' }).press('Enter');
await page.getByText('Support type *').click();
await page.getByRole('combobox', { name: 'Support type' }).selectOption('Introducing');

await page.getByRole('button', { name: 'Save objectives' }).click();
await page.getByRole('button', { name: 'Save and continue' }).click();
await page.getByRole('button', { name: 'Goals and objectives Complete' }).click();
Expand Down Expand Up @@ -888,11 +888,11 @@ test.describe('Activity Report', () => {
await activitySummary(page);

const p = page.waitForURL('**/goals-objectives');

// visit the goals & objectives page
await page.getByRole('button', { name: 'Goals and objectives Not Started' }).click();

await p;
await p;

// create the goal
await page.waitForTimeout(5000);
Expand All @@ -904,14 +904,14 @@ test.describe('Activity Report', () => {
// create the objective
await page.getByText('Select TTA objective *- Select -').click();
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');

await page.locator('[id="goalForEditing\.objectives\[0\]\.title"]').fill('Test objective for preserving objectives');
await blur(page);

await page.getByText('Topics *').click()
await page.keyboard.press('ArrowDown');
await page.keyboard.press('Enter');
await page.keyboard.press('Enter');

await page.getByRole('textbox', { name: /TTA provided for objective/i }).locator('div').nth(2).click();
await page.keyboard.type('An unlikely statement');
Expand Down