Skip to content

Commit

Permalink
fix(DatePicker): resolved page scrolling bug
Browse files Browse the repository at this point in the history
  • Loading branch information
thatblindgeye committed Jul 31, 2023
1 parent 8fa6757 commit 9907582
Show file tree
Hide file tree
Showing 5 changed files with 79 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -180,11 +180,8 @@ export const CalendarMonth = ({

useEffect(() => {
// Calendar month should not be focused on page load
// Datepicker should place focus in calendar month when opened
if ((shouldFocus || isDateFocused) && focusedDateValidated && focusRef.current) {
focusRef.current.focus();
} else {
setShouldFocus(true);
}
}, [focusedDate, isDateFocused, focusedDateValidated, focusRef]);

Check warning on line 186 in packages/react-core/src/components/CalendarMonth/CalendarMonth.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useEffect has a missing dependency: 'shouldFocus'. Either include it or remove the dependency array

Expand Down
9 changes: 8 additions & 1 deletion packages/react-core/src/components/DatePicker/DatePicker.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import * as React from 'react';
import { css } from '@patternfly/react-styles';
import styles from '@patternfly/react-styles/css/components/DatePicker/date-picker';
import buttonStyles from '@patternfly/react-styles/css/components/Button/button';
import calendarMonthStyles from '@patternfly/react-styles/css/components/CalendarMonth/calendar-month';
import { TextInput, TextInputProps } from '../TextInput/TextInput';
import { Popover, PopoverProps } from '../Popover/Popover';
import { InputGroup, InputGroupItem } from '../InputGroup';
Expand Down Expand Up @@ -197,9 +198,16 @@ const DatePickerBase = (
[setPopoverOpen, popoverOpen, selectOpen]

Check warning on line 198 in packages/react-core/src/components/DatePicker/DatePicker.tsx

View workflow job for this annotation

GitHub Actions / lint

React Hook useImperativeHandle has an unnecessary dependency: 'selectOpen'. Either exclude it or remove the dependency array
);

const createFocusSelectorString = (modifierClass: string) =>
`.${calendarMonthStyles.calendarMonthDatesCell}.${modifierClass} .${calendarMonthStyles.calendarMonthDate}`;
const focusSelectors = `${createFocusSelectorString(
calendarMonthStyles.modifiers.selected
)}, ${createFocusSelectorString(calendarMonthStyles.modifiers.current)}`;

return (
<div className={css(styles.datePicker, className)} ref={datePickerWrapperRef} style={style} {...props}>
<Popover
elementToFocus={focusSelectors}
position="bottom"
bodyContent={
<CalendarMonth
Expand All @@ -215,7 +223,6 @@ const DatePickerBase = (
dayFormat={dayFormat}
weekStart={weekStart}
rangeStart={rangeStart}
isDateFocused
/>
}
showClose={false}
Expand Down
19 changes: 18 additions & 1 deletion packages/react-core/src/components/Popover/Popover.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,10 @@ export interface PopoverProps {
closeBtnAriaLabel?: string;
/** Distance of the popover to its target. Defaults to 25. */
distance?: number;
/** The element to focus when the popover becomes visible. By default the first
* focusable element will receive focus.
*/
elementToFocus?: HTMLElement | SVGElement | string;
/**
* If true, tries to keep the popover in view by flipping it if necessary.
* If the position is set to 'auto', this prop is ignored.
Expand Down Expand Up @@ -266,6 +270,7 @@ export const Popover: React.FunctionComponent<PopoverProps> = ({
triggerRef,
hasNoPadding = false,
hasAutoWidth = false,
elementToFocus,
...rest
}: PopoverProps) => {
// could make this a prop in the future (true | false | 'toggle')
Expand All @@ -286,7 +291,7 @@ export const Popover: React.FunctionComponent<PopoverProps> = ({
React.useEffect(() => {
if (triggerManually) {
if (isVisible) {
show();
show(undefined, true);
} else {
hide();
}
Expand Down Expand Up @@ -390,13 +395,25 @@ export const Popover: React.FunctionComponent<PopoverProps> = ({
hide(event);
}
};

const content = (
<FocusTrap
ref={popoverRef}
active={focusTrapActive}
focusTrapOptions={{
returnFocusOnDeactivate: true,
clickOutsideDeactivates: true,
// FocusTrap's initialFocus can accept false as a value to prevent initial focus.
// We want to prevent this in case false is ever passed in.
initialFocus: elementToFocus || undefined,
checkCanFocusTrap: (containers) => new Promise((resolve) => {
const interval = setInterval(() => {
if (containers.every((container) => getComputedStyle(container).visibility !== 'hidden')) {
resolve();
clearInterval(interval);
}
}, 10);
}),
tabbableOptions: { displayCheck: 'none' },
fallbackFocus: () => {
// If the popover's trigger is focused but scrolled out of view,
Expand Down
22 changes: 20 additions & 2 deletions packages/react-core/src/components/Popover/examples/Popover.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,57 +19,75 @@ By default, the `appendTo` prop of the popover will append to the document body
### Basic

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

```

### Close popover from content (controlled)

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

```

### Close popover from content (uncontrolled)

Note: If you use the isVisible prop, either refer to the example above or if you want to use the hide callback from the content then be sure to keep isVisible in-sync.

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

```

### Without header/footer/close and no padding

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

```

### Width auto

Here the popover goes over the navigation, so the prop `appendTo` is set to the documents body.

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

```

### Popover react ref

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

```

### Popover selector ref

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

```

### Advanced

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

```

### Popover with icon in the title

Here the popover goes over the navigation, so the prop `appendTo` is set to the documents body.

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

```

### Alert popover

Here the popover goes over the navigation, so the prop `appendTo` is set to the documents body.

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

```

### Custom focus

Use the `elementToFocus` property to customize which element inside the Popover receives focus when opened.

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

```
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
import React from 'react';
import { Popover, Button } from '@patternfly/react-core';

export const PopoverCustomFocus: React.FunctionComponent = () => (
<Popover
elementToFocus="#popover-cancel-button"
showClose={false}
aria-label="Popover with custom focus"
headerContent={<div>Popover header</div>}
bodyContent={
<div>
Lorem ipsum dolor sit amet, consectetur adipisicing elit.{' '}
<Button
// Preventing default click behavior for example purposes only
onClick={(event: React.MouseEvent) => event.preventDefault()}
component="a"
isInline
variant="link"
href="#basic"
>
View the basic example
</Button>
</div>
}
footerContent={(hide) => (
<Button onClick={hide} variant="secondary" id="popover-cancel-button">
Cancel
</Button>
)}
>
<Button>Toggle popover with custom focus</Button>
</Popover>
);

0 comments on commit 9907582

Please sign in to comment.