Skip to content
Merged
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
3 changes: 2 additions & 1 deletion packages/react-core/src/components/Button/Button.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,7 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
iconPosition = 'start',
'aria-label': ariaLabel = null,
icon = null,
role,
ouiaId,
ouiaSafe = true,
tabIndex = null,
Expand Down Expand Up @@ -182,7 +183,7 @@ const ButtonBase: React.FunctionComponent<ButtonProps> = ({
disabled={isButtonElement ? isDisabled : null}
tabIndex={tabIndex !== null ? tabIndex : getDefaultTabIdx()}
type={isButtonElement || isInlineSpan ? type : null}
role={isInlineSpan ? 'button' : null}
role={isInlineSpan ? 'button' : role}
Copy link
Contributor

Choose a reason for hiding this comment

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

🥳

ref={innerRef}
{...ouiaProps}
>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,13 +11,15 @@ propComponents: ['CalendarMonth', 'CalendarFormat', 'CalendarMonthInlineProps']
### Selectable date

```ts file="./CalendarMonthSelectableDate.tsx"

```

### Date range

In this example, there are 2 dates selected: a range start date (via the `rangeStart` prop) and a range end date (via the `date` prop). Additionally, any dates prior to the range start date are disabled by passing in an array of functions to the `validators` prop. In this case a single function is passed in, which checks whether a date is greater than or equal to the range start date.

For this example, these dates are static and cannot be updated. For an interactive demo, see our [Date picker demos](https://www.patternfly.org/v4/components/date-picker/react-demos).
For this example, these dates are static and cannot be updated. For an interactive demo, see our [Date picker demos](/components/date-and-time/date-picker/react-demos).

```ts file="./CalendarMonthDateRange.tsx"

```
27 changes: 15 additions & 12 deletions packages/react-core/src/components/Menu/MenuItemAction.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@ import styles from '@patternfly/react-styles/css/components/Menu/menu';
import { css } from '@patternfly/react-styles';
import StarIcon from '@patternfly/react-icons/dist/esm/icons/star-icon';
import { MenuContext, MenuItemContext } from './MenuContext';

export interface MenuItemActionProps extends Omit<React.HTMLProps<HTMLButtonElement>, 'type' | 'ref'> {
import { Button } from '../Button';
export interface MenuItemActionProps extends React.HTMLProps<HTMLDivElement> {
/** Additional classes added to the action button */
className?: string;
/** The action icon to use */
Expand All @@ -24,7 +24,7 @@ export interface MenuItemActionProps extends Omit<React.HTMLProps<HTMLButtonElem
}

const MenuItemActionBase: React.FunctionComponent<MenuItemActionProps> = ({
className = '',
className,
icon,
onClick,
'aria-label': ariaLabel,
Expand All @@ -45,24 +45,27 @@ const MenuItemActionBase: React.FunctionComponent<MenuItemActionProps> = ({
onActionClick && onActionClick(event, itemId, actionId);
};
return (
<button
<div
className={css(
styles.menuItemAction,
isFavorited !== null && 'pf-m-favorite',
isFavorited && styles.modifiers.favorited,
className
)}
aria-label={ariaLabel}
onClick={onClickButton}
{...((isDisabled === true || isDisabledContext === true) && { disabled: true })}
ref={innerRef}
tabIndex={-1}
{...props}
>
<span className={css(styles.menuItemActionIcon)}>
<Button
aria-label={ariaLabel}
onClick={onClickButton}
ref={innerRef}
role="menuitem"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This should help resolve an aXe error that is related to #9968. This also matches the Core markup.

That said, it'd be worth looking into an alternative to making each button inside a menu a role="menuitem". My concern would be whether it's be totally clear that a user can navigate up/down and left/right inside a menu, or whether the assumption would be that there's only items in a vertical scope.

variant="plain"
tabIndex={-1}
isDisabled={isDisabled || isDisabledContext}
>
{icon === 'favorites' || isFavorited !== null ? <StarIcon aria-hidden /> : icon}
</span>
</button>
</Button>
</div>
);
}}
</MenuItemContext.Consumer>
Expand Down
20 changes: 13 additions & 7 deletions packages/react-core/src/helpers/KeyboardHandler.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -106,23 +106,30 @@ export const handleArrows = (

if (!activeRow.length || onlyTraverseSiblings) {
let nextSibling = activeElement;
// While a sibling exists, check each sibling to determine if it should be focussed

while (nextSibling) {
// Set the next checked sibling, determined by the horizontal arrow key direction
nextSibling = key === 'ArrowLeft' ? nextSibling.previousElementSibling : nextSibling.nextElementSibling;
const isDirectChildOfNavigableElement = nextSibling.parentElement === element;
const nextSiblingMainElement = isDirectChildOfNavigableElement ? nextSibling : nextSibling.parentElement;
nextSibling =
key === 'ArrowLeft'
? nextSiblingMainElement.previousElementSibling
: nextSiblingMainElement.nextElementSibling;

if (nextSibling) {
if (validSiblingTags.includes(nextSibling.tagName)) {
// If the sibling's tag is included in validSiblingTags, set the next target element and break the loop
moveTarget = nextSibling;
break;
}
// If the sibling's tag is not valid, skip to the next sibling if possible
// For cases where the validSiblingTag is inside a div wrapper
if (validSiblingTags.includes(nextSibling.children[0].tagName)) {
moveTarget = nextSibling.children[0];
break;
}
}
}
} else {
activeRow.forEach((focusableElement, index) => {
if (event.target === focusableElement) {
// Once found, move up or down the array by 1. Determined by the vertical arrow key direction
const increment = key === 'ArrowLeft' ? -1 : 1;
currentIndex = index + increment;
if (currentIndex >= activeRow.length) {
Expand All @@ -132,7 +139,6 @@ export const handleArrows = (
currentIndex = activeRow.length - 1;
}

// Set the next target element
moveTarget = activeRow[currentIndex];
}
});
Expand Down
4 changes: 2 additions & 2 deletions packages/react-integration/cypress/integration/menu.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,11 +57,11 @@ describe('Menu Test', () => {
});

it('Verify Menu with Actions', () => {
cy.get('#actions-list.pf-v6-c-menu__list > li > button').last().should('have.class', 'pf-v6-c-menu__item-action');
cy.get('#actions-list.pf-v6-c-menu__list > li > div').last().should('have.class', 'pf-v6-c-menu__item-action');
});

it('Verify Menu with Favorites', () => {
cy.get('#favorites-menu .pf-v6-c-menu__item-action[aria-label="not starred"]').first().click();
cy.get('#favorites-menu .pf-v6-c-menu__item-action > button[aria-label="not starred"]').first().click();

cy.get('#favorites-menu.pf-v6-c-menu > section').first().should('contain', 'Favorites');
});
Expand Down