Skip to content

chore: graduate primer_react_action_menu_display_in_viewport_inside_dialog #7700

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

Draft
francinelucca wants to merge 1 commit into
base: main
Choose a base branch
from
+116 −204
Draft

chore: graduate primer_react_action_menu_display_in_viewport_inside_dialog #7700

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
161 changes: 79 additions & 82 deletions packages/react/src/ActionMenu/ActionMenu.examples.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
} from '@primer/octicons-react'
import type {AnchorPosition, AnchorSide} from '@primer/behaviors'
import classes from './ActionMenu.examples.stories.module.css'
import {FeatureFlags} from '../FeatureFlags'

Check failure on line 24 in packages/react/src/ActionMenu/ActionMenu.examples.stories.tsx

View workflow job for this annotation

GitHub Actions / lint 

'FeatureFlags' is defined but never used

export default {
title: 'Components/ActionMenu/Examples',
Expand Down Expand Up @@ -673,90 +673,87 @@
))

return (
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: true}}>
<div style={{padding: '20px'}}>
{/* Main scrollable content */}
<div>
<Text as="h1" style={{marginBottom: '16px'}}>
Main Page Content
</Text>
<div style={{padding: '20px'}}>
{/* Main scrollable content */}
<div>
<Text as="h1" style={{marginBottom: '16px'}}>
Main Page Content
</Text>

<Button onClick={openDialog} style={{margin: '16px 0'}}>
Open Dialog with ActionMenu
</Button>

<Button onClick={openDialog} style={{margin: '16px 0'}}>
Open Dialog with ActionMenu
</Button>

{/* Show more content after the button to make it scrollable */}
{scrollableContent}
</div>

{/* Dialog containing ActionMenu */}
{isDialogOpen && (
<Dialog title="Dialog with ActionMenu" onClose={onDialogClose} width="medium">
<Text as="p" style={{marginBottom: '12px'}}>
This dialog contains an ActionMenu. The main page content behind is long enough to be scrollable.
</Text>

<Text as="h3" style={{marginBottom: '8px', fontWeight: '600'}}>
Document Settings
</Text>

<Text as="p" style={{marginBottom: '16px', color: '#656d76'}}>
Configure the document properties and sharing settings. These options allow you to control how the
document is displayed and who has access to it.
</Text>

<ActionMenu>
<ActionMenu.Button>Actions</ActionMenu.Button>
<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Item onSelect={() => alert('Save clicked')}>
Save
<ActionList.TrailingVisual>⌘S</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Save as clicked')}>
Save as...
<ActionList.TrailingVisual>⌘⇧S</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Export clicked')}>
Export
<ActionList.TrailingVisual>⌘E</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Print clicked')}>
Print
<ActionList.TrailingVisual>⌘P</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Copy clicked')}>
Copy
<ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Paste clicked')}>
Paste
<ActionList.TrailingVisual>⌘V</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Duplicate clicked')}>
Duplicate
<ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Share clicked')}>
Share
<ActionList.TrailingVisual>⌘⇧U</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Share via email clicked')}>Share via email</ActionList.Item>
<ActionList.Item onSelect={() => alert('Share via link clicked')}>Share via link</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>

<Text as="p" style={{marginTop: '12px'}}>
You can interact with the ActionMenu above while the main page content remains scrollable in the
background.
</Text>
</Dialog>
)}
{/* Show more content after the button to make it scrollable */}
{scrollableContent}
</div>
</FeatureFlags>

{/* Dialog containing ActionMenu */}
{isDialogOpen && (
<Dialog title="Dialog with ActionMenu" onClose={onDialogClose} width="medium">
<Text as="p" style={{marginBottom: '12px'}}>
This dialog contains an ActionMenu. The main page content behind is long enough to be scrollable.
</Text>

<Text as="h3" style={{marginBottom: '8px', fontWeight: '600'}}>
Document Settings
</Text>

<Text as="p" style={{marginBottom: '16px', color: '#656d76'}}>
Configure the document properties and sharing settings. These options allow you to control how the document
is displayed and who has access to it.
</Text>

<ActionMenu>
<ActionMenu.Button>Actions</ActionMenu.Button>
<ActionMenu.Overlay width="medium">
<ActionList>
<ActionList.Item onSelect={() => alert('Save clicked')}>
Save
<ActionList.TrailingVisual>⌘S</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Save as clicked')}>
Save as...
<ActionList.TrailingVisual>⌘⇧S</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Export clicked')}>
Export
<ActionList.TrailingVisual>⌘E</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Print clicked')}>
Print
<ActionList.TrailingVisual>⌘P</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Copy clicked')}>
Copy
<ActionList.TrailingVisual>⌘C</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Paste clicked')}>
Paste
<ActionList.TrailingVisual>⌘V</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Duplicate clicked')}>
Duplicate
<ActionList.TrailingVisual>⌘D</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Divider />
<ActionList.Item onSelect={() => alert('Share clicked')}>
Share
<ActionList.TrailingVisual>⌘⇧U</ActionList.TrailingVisual>
</ActionList.Item>
<ActionList.Item onSelect={() => alert('Share via email clicked')}>Share via email</ActionList.Item>
<ActionList.Item onSelect={() => alert('Share via link clicked')}>Share via link</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>

<Text as="p" style={{marginTop: '12px'}}>
You can interact with the ActionMenu above while the main page content remains scrollable in the background.
</Text>
</Dialog>
)}
</div>
)
}

Expand Down
150 changes: 36 additions & 114 deletions packages/react/src/ActionMenu/ActionMenu.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -904,55 +904,19 @@ describe('ActionMenu', () => {
})
})

describe('feature flag: primer_react_action_menu_display_in_viewport_inside_dialog', () => {
describe('displayInViewport behavior', () => {
const mockGetAnchoredPosition = vi.mocked(getAnchoredPosition)

beforeEach(() => {
// Reset mock before each test
mockGetAnchoredPosition.mockClear()
})

it('should enable displayInViewport when flag is enabled and ActionMenu is inside a dialog', async () => {
it('should enable displayInViewport when ActionMenu is inside a dialog', async () => {
// When the ActionMenu is wrapped in a Dialog, it's inside a dialog context.
// With the flag enabled, displayInViewport should be automatically enabled.
// displayInViewport should be automatically enabled.
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: true}}>
<Dialog onClose={() => {}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Dialog>
</FeatureFlags>,
)

const user = userEvent.setup()
const button = component.getByRole('button', {name: 'Toggle Menu'})
await user.click(button)

await waitFor(() => {
expect(component.queryByRole('menu')).toBeInTheDocument()
})

// Verify getAnchoredPosition was called with displayInViewport: true
await waitFor(() => {
expect(mockGetAnchoredPosition).toHaveBeenCalled()
})

const calls = mockGetAnchoredPosition.mock.calls
const lastCall = calls[calls.length - 1]
expect(lastCall[2]?.displayInViewport).toBe(true)
})

it('should not enable displayInViewport when flag is enabled but ActionMenu is NOT inside a dialog', async () => {
// Without being wrapped in a Dialog, the ActionMenu is not in a dialog context.
// Even with the flag enabled, displayInViewport should remain at its default (false/undefined).
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: true}}>
<Dialog onClose={() => {}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
Expand All @@ -961,47 +925,43 @@ describe('ActionMenu', () => {
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</FeatureFlags>,
</Dialog>,
)

const user = userEvent.setup()
const button = component.getByRole('button')
const button = component.getByRole('button', {name: 'Toggle Menu'})
await user.click(button)

await waitFor(() => {
expect(component.queryByRole('menu')).toBeInTheDocument()
})

// Verify getAnchoredPosition was called without displayInViewport enabled
// Verify getAnchoredPosition was called with displayInViewport: true
await waitFor(() => {
expect(mockGetAnchoredPosition).toHaveBeenCalled()
})

const calls = mockGetAnchoredPosition.mock.calls
const lastCall = calls[calls.length - 1]
expect(lastCall[2]?.displayInViewport).not.toBe(true)
expect(lastCall[2]?.displayInViewport).toBe(true)
})

it('should not enable displayInViewport when flag is disabled, even inside a dialog', async () => {
// Even when inside a Dialog, with the flag disabled, displayInViewport
// should remain at its default (false/undefined).
it('should not enable displayInViewport when ActionMenu is NOT inside a dialog', async () => {
// Without being wrapped in a Dialog, the ActionMenu is not in a dialog context.
// displayInViewport should remain at its default (false/undefined).
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: false}}>
<Dialog onClose={() => {}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Dialog>
</FeatureFlags>,
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>,
)

const user = userEvent.setup()
const button = component.getByRole('button', {name: 'Toggle Menu'})
const button = component.getByRole('button')
await user.click(button)

await waitFor(() => {
Expand All @@ -1018,56 +978,20 @@ describe('ActionMenu', () => {
expect(lastCall[2]?.displayInViewport).not.toBe(true)
})

it('should not enable displayInViewport when flag is disabled and outside dialog', async () => {
// Default scenario: flag disabled and not in a dialog context.
// displayInViewport should remain at its default (false/undefined).
it('should respect explicit displayInViewport prop over default logic', async () => {
// Test that an explicit displayInViewport=false prop overrides the automatic
// detection, even when the ActionMenu is inside a dialog.
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: false}}>
<Dialog onClose={() => {}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay>
<ActionMenu.Overlay displayInViewport={false}>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</FeatureFlags>,
)

const user = userEvent.setup()
const button = component.getByRole('button')
await user.click(button)

await waitFor(() => {
expect(component.queryByRole('menu')).toBeInTheDocument()
})

// Verify getAnchoredPosition was called without displayInViewport enabled
await waitFor(() => {
expect(mockGetAnchoredPosition).toHaveBeenCalled()
})

const calls = mockGetAnchoredPosition.mock.calls
const lastCall = calls[calls.length - 1]
expect(lastCall[2]?.displayInViewport).not.toBe(true)
})

it('should respect explicit displayInViewport prop over feature flag logic', async () => {
// Test that an explicit displayInViewport=false prop overrides the automatic
// detection, even when the flag is enabled and the ActionMenu is inside a dialog.
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: true}}>
<Dialog onClose={() => {}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay displayInViewport={false}>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</Dialog>
</FeatureFlags>,
</Dialog>,
)

const user = userEvent.setup()
Expand All @@ -1088,20 +1012,18 @@ describe('ActionMenu', () => {
expect(lastCall[2]?.displayInViewport).toBe(false)
})

it('should respect explicit displayInViewport=true prop even when flag is disabled', async () => {
it('should respect explicit displayInViewport=true prop', async () => {
// Test that an explicit displayInViewport=true prop works regardless of
// the flag state or dialog context.
// the dialog context.
const component = HTMLRender(
<FeatureFlags flags={{primer_react_action_menu_display_in_viewport_inside_dialog: false}}>
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay displayInViewport={true}>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>
</FeatureFlags>,
<ActionMenu>
<ActionMenu.Button>Toggle Menu</ActionMenu.Button>
<ActionMenu.Overlay displayInViewport={true}>
<ActionList>
<ActionList.Item>New file</ActionList.Item>
</ActionList>
</ActionMenu.Overlay>
</ActionMenu>,
)

const user = userEvent.setup()
Expand Down
Loading
Loading