Skip to content

Commit

Permalink
Fixed EuiSuperDatePicker input focus via EuiTabbedContent. (#3154)
Browse files Browse the repository at this point in the history
* fixed bug in input focus

* added cl

* Fixed InitializeFocus stealing focus in EuiTabbedContent

* moved onFocus to EuiTabs

* updated jest snapshots

* retain ref in div and forward ref in EuiTab

* updated jest snapshot

* removed ref in div

* renamed divRef to tabsRef

* updated cl

* Update CHANGELOG.md

Co-authored-by: Caroline Horn <549577+cchaos@users.noreply.github.com>
  • Loading branch information
ashikmeerankutty and cchaos authored Apr 7, 2020
1 parent 41c88fb commit c7edcfd
Show file tree
Hide file tree
Showing 4 changed files with 53 additions and 41 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@

**Bug Fixes**

- Fixed the `initialSelectedTab` prop of `EuiTabbedContent` to not steal focus from content. Which fixed the bug in `EuiSuperDatePicker` that required two clicks to focus input in relative tab ([#3154](https://github.com/elastic/eui/pull/3154))
- Fixed the `img` element in `EuiIcon` using custom SVGs to have an `alt` attribute with an empty string, rather than no `alt` attribute at all ([#3245](https://github.com/elastic/eui/pull/3245))
- Added overflows to EuiDataGrid toolbar dropdowns when there are many columns ([#3238](https://github.com/elastic/eui/pull/3238))
- Fixed `EuiIcon`'s icon `type` definition to allow custom React components ([#3252](https://github.com/elastic/eui/pull/3252))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ exports[`EuiTabbedContent behavior when uncontrolled, the selected tab should up
]
}
>
<div
onFocus={[Function]}
>
<EuiTabs>
<div>
<ForwardRef
onFocus={[Function]}
>
<div
className="euiTabs"
onFocus={[Function]}
role="tablist"
>
<EuiTab
Expand Down Expand Up @@ -134,7 +135,7 @@ exports[`EuiTabbedContent behavior when uncontrolled, the selected tab should up
</button>
</EuiTab>
</div>
</EuiTabs>
</ForwardRef>
<div
aria-labelledby="kibana"
id="42"
Expand Down
25 changes: 13 additions & 12 deletions src/components/tabs/tabbed_content/tabbed_content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ export class EuiTabbedContent extends Component<

private readonly rootId = htmlIdGenerator()();

private readonly divRef = createRef<HTMLDivElement>();
private readonly tabsRef = createRef<HTMLDivElement>();

constructor(props: EuiTabbedContentProps) {
super(props);
Expand All @@ -89,19 +89,19 @@ export class EuiTabbedContent extends Component<
componentDidMount() {
// IE11 doesn't support the `relatedTarget` event property for blur events
// but does add it for focusout. React doesn't support `onFocusOut` so here we are.
if (this.divRef.current) {
if (this.tabsRef.current) {
// Current short-term solution for event listener (see https://github.com/elastic/eui/pull/2717)
this.divRef.current.addEventListener(
this.tabsRef.current.addEventListener(
'focusout' as 'blur',
this.removeFocus
);
}
}

componentWillUnmount() {
if (this.divRef.current) {
if (this.tabsRef.current) {
// Current short-term solution for event listener (see https://github.com/elastic/eui/pull/2717)
this.divRef.current.removeEventListener(
this.tabsRef.current.removeEventListener(
'focusout' as 'blur',
this.removeFocus
);
Expand All @@ -113,7 +113,7 @@ export class EuiTabbedContent extends Component<
// Must wait for setState to finish before calling `.focus()`
// as the focus call triggers a blur on the first tab
this.setState({ inFocus: true }, () => {
const targetTab: HTMLDivElement | null = this.divRef.current!.querySelector(
const targetTab: HTMLDivElement | null = this.tabsRef.current!.querySelector(
`#${this.state.selectedTabId}`
);
targetTab!.focus();
Expand Down Expand Up @@ -170,12 +170,13 @@ export class EuiTabbedContent extends Component<
const { content: selectedTabContent, id: selectedTabId } = selectedTab!;

return (
<div
ref={this.divRef}
className={className}
{...rest}
onFocus={this.initializeFocus}>
<EuiTabs expand={expand} display={display} size={size}>
<div className={className} {...rest}>
<EuiTabs
ref={this.tabsRef}
expand={expand}
display={display}
size={size}
onFocus={this.initializeFocus}>
{tabs.map((tab: EuiTabbedContentTab) => {
const {
id,
Expand Down
57 changes: 33 additions & 24 deletions src/components/tabs/tabs.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,37 @@ export type EuiTabsProps = CommonProps &
size?: EuiTabsSizes;
};

export const EuiTabs = ({
children,
className,

display = 'default',
expand = false,
size = 'm',
...rest
}: PropsWithChildren<EuiTabsProps>) => {
const classes = classNames(
'euiTabs',
displayToClassNameMap[display],
sizeToClassNameMap[size],
export type EuiTabRef = HTMLDivElement;

export const EuiTabs = React.forwardRef<
EuiTabRef,
PropsWithChildren<EuiTabsProps>
>(
(
{
'euiTabs--expand': expand,
},
className
);

return (
<div role="tablist" className={classes} {...rest}>
{children}
</div>
);
};
children,
className,
display = 'default',
expand = false,
size = 'm',
...rest
}: PropsWithChildren<EuiTabsProps>,
ref
) => {
const classes = classNames(
'euiTabs',
displayToClassNameMap[display],
sizeToClassNameMap[size],
{
'euiTabs--expand': expand,
},
className
);

return (
<div ref={ref} role="tablist" className={classes} {...rest}>
{children}
</div>
);
}
);

0 comments on commit c7edcfd

Please sign in to comment.