Skip to content

fix(ResponsiveActions): Disable kebab when all actions are disabled #928

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

Open
rhamilto wants to merge 3 commits into
base: main
Choose a base branch
from
+130 −25
Open

fix(ResponsiveActions): Disable kebab when all actions are disabled #928

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8062a9d
fix(ResponsiveActions): Disable kebab when all actions are disabled
rhamilto May 6, 2026
52cb9a6
refactor: remove unnecessary comments from ResponsiveActions
rhamilto May 8, 2026
dbd6753
test(ResponsiveActions): use RTL conventions and remove unnecessary s…
rhamilto May 8, 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
68 changes: 67 additions & 1 deletion packages/module/src/ResponsiveActions/ResponsiveActions.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { render } from '@testing-library/react';
import { render, screen } from '@testing-library/react';
import ResponsiveActions from './ResponsiveActions';
import ResponsiveAction from '../ResponsiveAction';

Expand Down Expand Up @@ -56,5 +56,71 @@ describe('ResponsiveActions component', () => {
expect(buttons).toHaveLength(2);
expect(container).toMatchSnapshot();
});

test('ResponsiveActions with all dropdown items disabled should disable kebab', () => {
render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isDisabled>Disabled action 1</ResponsiveAction>
<ResponsiveAction isDisabled>Disabled action 2</ResponsiveAction>
</ResponsiveActions>);

const kebabToggle = screen.getByRole('button', { name: /actions overflow menu/i });
expect(kebabToggle).toBeDisabled();
});

test('ResponsiveActions with some enabled dropdown items should not disable kebab', () => {
render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isDisabled>Disabled action</ResponsiveAction>
<ResponsiveAction>Enabled action</ResponsiveAction>
</ResponsiveActions>);

const kebabToggle = screen.getByRole('button', { name: /actions overflow menu/i });
expect(kebabToggle).toBeEnabled();
});

test('ResponsiveActions with enabled pinned item and disabled regular item should disable kebab above breakpoint', () => {
render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isPinned>Enabled pinned action</ResponsiveAction>
<ResponsiveAction isDisabled>Disabled regular action</ResponsiveAction>
</ResponsiveActions>);

const kebabToggle = screen.getByRole('button', { name: /actions overflow menu/i });
expect(kebabToggle).toBeDisabled();
});

test('ResponsiveActions with enabled pinned item and enabled regular item should not disable kebab', () => {
render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isPinned>Enabled pinned action</ResponsiveAction>
<ResponsiveAction>Enabled regular action</ResponsiveAction>
</ResponsiveActions>);

const kebabToggle = screen.getByRole('button', { name: /actions overflow menu/i });
expect(kebabToggle).toBeEnabled();
});

test('ResponsiveActions with all dropdown items disabled including pinned should disable kebab', () => {
render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isPinned isDisabled>Disabled pinned action</ResponsiveAction>
<ResponsiveAction isDisabled>Disabled action</ResponsiveAction>
</ResponsiveActions>);

const kebabToggle = screen.getByRole('button', { name: /actions overflow menu/i });
expect(kebabToggle).toBeDisabled();
});

test('ResponsiveActions with only persistent items should not render kebab', () => {
const { container } = render(
<ResponsiveActions breakpoint="lg">
<ResponsiveAction isPersistent>Persistent action</ResponsiveAction>
</ResponsiveActions>);

// Should not have kebab when only persistent items exist
const kebabToggle = container.querySelector('[data-ouia-component-id="ResponsiveActions-menu-dropdown-toggle"]');
expect(kebabToggle).toBeNull();
});
});
});
87 changes: 63 additions & 24 deletions packages/module/src/ResponsiveActions/ResponsiveActions.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,9 @@
import type { ReactNode, FunctionComponent } from 'react';
import { Children, isValidElement, useState } from 'react';
import { Children, isValidElement, useState, useContext } from 'react';
import { Button, Dropdown, DropdownList, MenuToggle, OverflowMenu, OverflowMenuContent, OverflowMenuControl, OverflowMenuDropdownItem, OverflowMenuGroup, OverflowMenuItem, OverflowMenuProps } from '@patternfly/react-core';
import { EllipsisVIcon } from '@patternfly/react-icons';
import { ResponsiveActionProps } from '../ResponsiveAction';
import { OverflowMenuContext } from '@patternfly/react-core/dist/esm/components/OverflowMenu/OverflowMenuContext';
Copy link
Copy Markdown
Collaborator

@wise-king-sullyman wise-king-sullyman May 8, 2026

Choose a reason for hiding this comment

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

Hm, with this not being something we explicitly export from react-core or document as part of our public API I'm not sure about importing it here. I would definitely be against it in a product, but maybe it's ok to do stuff like that in an extension? WDYT @kmcfaul @thatblindgeye ?

Copy link
Copy Markdown
Member Author

@rhamilto rhamilto May 8, 2026 •
edited
Loading

Choose a reason for hiding this comment

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

I will argue it or something like it should be exported. It reduces the need for duplication. For example, I am using it in OpenShift console because to my knowledge, PatternFly doesn't offer a comparable solution where a menu toggle changes based on viewport. Perhaps this is just a gap in PatternFly. But by exposing this, it frees up developers to solve for problems PatternFly doesn't cover using PatternFly tools.


/** extends OverflowMenuProps */
export interface ResponsiveActionsProps extends Omit<OverflowMenuProps, 'ref' | 'breakpoint'> {
Expand All @@ -14,13 +15,62 @@ export interface ResponsiveActionsProps extends Omit<OverflowMenuProps, 'ref' |
children: React.ReactNode;
}

export const ResponsiveActions: FunctionComponent<ResponsiveActionsProps> = ({ ouiaId = 'ResponsiveActions', breakpoint = 'lg', children, ...props }: ResponsiveActionsProps) => {
const ResponsiveActionsDropdown: FunctionComponent<{
ouiaId: string;
dropdownItems: ReactNode[];
pinnedItemsDisabled: boolean[];
regularItemsDisabled: boolean[];
}> = ({ ouiaId, dropdownItems, pinnedItemsDisabled, regularItemsDisabled }) => {
const [ isOpen, setIsOpen ] = useState(false);
const { isBelowBreakpoint } = useContext(OverflowMenuContext);

const isKebabDisabled = (() => {
const allPinnedDisabled = pinnedItemsDisabled.length > 0 && pinnedItemsDisabled.every(disabled => disabled);
const allRegularDisabled = regularItemsDisabled.length > 0 && regularItemsDisabled.every(disabled => disabled);

if (isBelowBreakpoint) {
return (pinnedItemsDisabled.length > 0 || regularItemsDisabled.length > 0) &&
(pinnedItemsDisabled.length === 0 || allPinnedDisabled) &&
(regularItemsDisabled.length === 0 || allRegularDisabled);
} else {
return allRegularDisabled;
}
})();

return (
<Dropdown
ouiaId={`${ouiaId}-menu-dropdown`}
onSelect={() => setIsOpen(false)}
toggle={(toggleRef) => (
<MenuToggle
ouiaId={`${ouiaId}-menu-dropdown-toggle`}
ref={toggleRef}
aria-label="Actions overflow menu"
variant="plain"
icon={<EllipsisVIcon />}
onClick={() => setIsOpen(!isOpen)}
isExpanded={isOpen}
isDisabled={isKebabDisabled}
/>
)}
isOpen={isOpen}
onOpenChange={setIsOpen}
>
<DropdownList data-ouia-component-id={`${ouiaId}-menu-dropdown-list`}>
{dropdownItems}
</DropdownList>
</Dropdown>
);
};

export const ResponsiveActions: FunctionComponent<ResponsiveActionsProps> = ({ ouiaId = 'ResponsiveActions', breakpoint = 'lg', children, ...props }: ResponsiveActionsProps) => {

// separate persistent, pinned and collapsed actions
const persistentActions: ReactNode[] = [];
const pinnedActions: ReactNode[] = [];
const dropdownItems: ReactNode[] = [];
const pinnedItemsDisabled: boolean[] = [];
const regularItemsDisabled: boolean[] = [];
let hasRegularActions = false;

Children.forEach(children, (child, index) => {
Expand All @@ -37,7 +87,6 @@ export const ResponsiveActions: FunctionComponent<ResponsiveActionsProps> = ({ o
</OverflowMenuItem>
);
} else {
// Track if there are any regular (non-persistent, non-pinned) actions
hasRegularActions = true;
}

Expand All @@ -47,6 +96,11 @@ export const ResponsiveActions: FunctionComponent<ResponsiveActionsProps> = ({ o
{children}
</OverflowMenuDropdownItem>
);
if (isPinned) {
pinnedItemsDisabled.push(!!actionProps.isDisabled);
} else {
regularItemsDisabled.push(!!actionProps.isDisabled);
}
}
}
});
Expand Down Expand Up @@ -74,27 +128,12 @@ export const ResponsiveActions: FunctionComponent<ResponsiveActionsProps> = ({ o
) : null}
{dropdownItems.length > 0 && (
<OverflowMenuControl hasAdditionalOptions={hasRegularActions} data-ouia-component-id={`${ouiaId}-menu-control`}>
<Dropdown
ouiaId={`${ouiaId}-menu-dropdown`}
onSelect={() => setIsOpen(false)}
toggle={(toggleRef) => (
<MenuToggle
ouiaId={`${ouiaId}-menu-dropdown-toggle`}
ref={toggleRef}
aria-label="Actions overflow menu"
variant="plain"
icon={<EllipsisVIcon />}
onClick={() => setIsOpen(!isOpen)}
isExpanded={isOpen}
/>
)}
isOpen={isOpen}
onOpenChange={setIsOpen}
>
<DropdownList data-ouia-component-id={`${ouiaId}-menu-dropdown-list`}>
{dropdownItems}
</DropdownList>
</Dropdown>
<ResponsiveActionsDropdown
ouiaId={ouiaId}
dropdownItems={dropdownItems}
pinnedItemsDisabled={pinnedItemsDisabled}
regularItemsDisabled={regularItemsDisabled}
/>
</OverflowMenuControl>
)}
</OverflowMenu>
Expand Down
Loading