Skip to content
This repository has been archived by the owner on Sep 11, 2024. It is now read-only.

Commit

Permalink
Switch AccessibleButton and derivatives to using forwardRef (#12054)
Browse files Browse the repository at this point in the history
* Prevent Cypress typechecking react-sdk components without strict mode

This prevented us from switching to `forwardRef` in a bunch of places
due to it behaving different with & without strict mode.

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Update global.d.ts

* Switch AccessibleButton and derivatives to using `forwardRef`

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Add missing ref={ref}

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Ensure RefObjects are used consistently

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Re-add WysiwygAutocomplete displayname

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Fix forwardRef types

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Add comments

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Remove unused export

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Readd comment

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate types & comments

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Apply suggestions from code review

Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>

* Add comment

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Iterate

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

* Improve comment

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>

---------

Signed-off-by: Michael Telatynski <7t3chguy@gmail.com>
Co-authored-by: Richard van der Hoff <1389908+richvdh@users.noreply.github.com>
  • Loading branch information
t3chguy and richvdh authored Dec 21, 2023
1 parent 0a881e2 commit f632e21
Show file tree
Hide file tree
Showing 37 changed files with 102 additions and 95 deletions.
17 changes: 7 additions & 10 deletions src/accessibility/context_menu/ContextMenuButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { ComponentProps } from "react";
import React, { ComponentProps, forwardRef, Ref } from "react";

import AccessibleButton from "../../components/views/elements/AccessibleButton";

Expand All @@ -27,14 +27,10 @@ type Props<T extends keyof JSX.IntrinsicElements> = ComponentProps<typeof Access
};

// Semantic component for representing the AccessibleButton which launches a <ContextMenu />
export const ContextMenuButton = <T extends keyof JSX.IntrinsicElements>({
label,
isExpanded,
children,
onClick,
onContextMenu,
...props
}: Props<T>): JSX.Element => {
export const ContextMenuButton = forwardRef(function <T extends keyof JSX.IntrinsicElements>(
{ label, isExpanded, children, onClick, onContextMenu, ...props }: Props<T>,
ref: Ref<HTMLElement>,
) {
return (
<AccessibleButton
{...props}
Expand All @@ -44,8 +40,9 @@ export const ContextMenuButton = <T extends keyof JSX.IntrinsicElements>({
aria-label={label}
aria-haspopup={true}
aria-expanded={isExpanded}
ref={ref}
>
{children}
</AccessibleButton>
);
};
});
16 changes: 7 additions & 9 deletions src/accessibility/context_menu/ContextMenuTooltipButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { ComponentProps } from "react";
import React, { ComponentProps, forwardRef, Ref } from "react";

import AccessibleTooltipButton from "../../components/views/elements/AccessibleTooltipButton";

Expand All @@ -26,13 +26,10 @@ type Props<T extends keyof JSX.IntrinsicElements> = ComponentProps<typeof Access
};

// Semantic component for representing the AccessibleButton which launches a <ContextMenu />
export const ContextMenuTooltipButton = <T extends keyof JSX.IntrinsicElements>({
isExpanded,
children,
onClick,
onContextMenu,
...props
}: Props<T>): JSX.Element => {
export const ContextMenuTooltipButton = forwardRef(function <T extends keyof JSX.IntrinsicElements>(
{ isExpanded, children, onClick, onContextMenu, ...props }: Props<T>,
ref: Ref<HTMLElement>,
) {
return (
<AccessibleTooltipButton
{...props}
Expand All @@ -41,8 +38,9 @@ export const ContextMenuTooltipButton = <T extends keyof JSX.IntrinsicElements>(
aria-haspopup={true}
aria-expanded={isExpanded}
forceHide={isExpanded}
ref={ref}
>
{children}
</AccessibleTooltipButton>
);
};
});
2 changes: 1 addition & 1 deletion src/accessibility/roving/RovingAccessibleButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const RovingAccessibleButton = <T extends keyof JSX.IntrinsicElements>({
if (focusOnMouseOver) onFocusInternal();
onMouseOver?.(event);
}}
inputRef={ref}
ref={ref}
tabIndex={isActive ? 0 : -1}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion src/accessibility/roving/RovingAccessibleTooltipButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export const RovingAccessibleTooltipButton = <T extends keyof JSX.IntrinsicEleme
onFocusInternal();
onFocus?.(event);
}}
inputRef={ref}
ref={ref}
tabIndex={isActive ? 0 : -1}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/GenericDropdownMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ export function GenericDropdownMenu<T>({
<>
<ContextMenuButton
className="mx_GenericDropdownMenu_button"
inputRef={button}
ref={button}
isExpanded={menuDisplayed}
onClick={(ev: ButtonEvent) => {
openMenu();
Expand Down
5 changes: 3 additions & 2 deletions src/components/structures/SpaceHierarchy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ limitations under the License.
*/

import React, {
ComponentProps,
Dispatch,
KeyboardEvent,
KeyboardEventHandler,
Expand Down Expand Up @@ -349,7 +350,7 @@ const Tile: React.FC<ITileProps> = ({
})}
onClick={hasPermissions && onToggleClick ? onToggleClick : onPreviewClick}
onKeyDown={onKeyDown}
inputRef={ref}
ref={ref}
onFocus={onFocus}
tabIndex={isActive ? 0 : -1}
>
Expand Down Expand Up @@ -664,7 +665,7 @@ const ManageButtons: React.FC<IManageButtonsProps> = ({ hierarchy, selected, set
const disabled = !selectedRelations.length || removing || saving;

let Button: React.ComponentType<React.ComponentProps<typeof AccessibleButton>> = AccessibleButton;
let props = {};
let props: Partial<ComponentProps<typeof AccessibleTooltipButton>> = {};
if (!selectedRelations.length) {
Button = AccessibleTooltipButton;
props = {
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/SpaceRoomView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ const SpaceLandingAddButton: React.FC<{ space: Room }> = ({ space }) => {
<>
<ContextMenuButton
kind="primary"
inputRef={handle}
ref={handle}
onClick={openMenu}
isExpanded={menuDisplayed}
label={_t("action|add")}
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/ThreadPanel.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@ export const ThreadPanelHeader: React.FC<{
<>
<ContextMenuButton
className="mx_ThreadPanel_dropdown"
inputRef={button}
ref={button}
isExpanded={menuDisplayed}
onClick={(ev: ButtonEvent) => {
openMenu();
Expand Down
2 changes: 1 addition & 1 deletion src/components/structures/UserMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -453,7 +453,7 @@ export default class UserMenu extends React.Component<IProps, IState> {
<ContextMenuButton
className="mx_UserMenu_contextMenuButton"
onClick={this.onOpenMenuClick}
inputRef={this.buttonRef}
ref={this.buttonRef}
label={_t("a11y|user_menu")}
isExpanded={!!this.state.contextMenuPosition}
onContextMenu={this.onContextMenu}
Expand Down
5 changes: 4 additions & 1 deletion src/components/views/audio_messages/PlayPauseButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ import AccessibleTooltipButton from "../elements/AccessibleTooltipButton";
import { _t } from "../../../languageHandler";
import { Playback, PlaybackState } from "../../../audio/Playback";

type Props = Omit<ComponentProps<typeof AccessibleTooltipButton>, "title" | "onClick" | "disabled" | "element"> & {
type Props = Omit<
ComponentProps<typeof AccessibleTooltipButton>,
"title" | "onClick" | "disabled" | "element" | "ref"
> & {
// Playback instance to manipulate. Cannot change during the component lifecycle.
playback: Playback;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -974,7 +974,7 @@ export class FallbackAuthEntry extends React.Component<IAuthEntryProps> {
}
return (
<div>
<AccessibleButton kind="link" inputRef={this.fallbackButton} onClick={this.onShowFallbackClick}>
<AccessibleButton kind="link" ref={this.fallbackButton} onClick={this.onShowFallbackClick}>
{_t("auth|uia|fallback_button")}
</AccessibleButton>
{errorSection}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/context_menus/KebabContextMenu.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const KebabContextMenu: React.FC<KebabContextMenuProps> = ({ options, tit

return (
<>
<ContextMenuButton {...props} onClick={openMenu} title={title} isExpanded={menuDisplayed} inputRef={button}>
<ContextMenuButton {...props} onClick={openMenu} title={title} isExpanded={menuDisplayed} ref={button}>
<ContextMenuIcon className="mx_KebabContextMenu_icon" />
</ContextMenuButton>
{menuDisplayed && (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ const ThreadListContextMenu: React.FC<ThreadListContextMenuProps> = ({
onClick={openMenu}
title={_t("right_panel|thread_list|context_menu_label")}
isExpanded={menuDisplayed}
inputRef={button}
ref={button}
data-testid="threadlist-dropdown-button"
/>
{menuDisplayed && (
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/dialogs/spotlight/Option.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export const Option: React.FC<OptionProps> = ({ inputRef, children, endAdornment
{...props}
className={classNames(className, "mx_SpotlightDialog_option")}
onFocus={onFocus}
inputRef={ref}
ref={ref}
tabIndex={-1}
aria-selected={isActive}
role="option"
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/dialogs/spotlight/TooltipOption.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ export const TooltipOption: React.FC<TooltipOptionProps> = ({ inputRef, classNam
{...props}
className={classNames(className, "mx_SpotlightDialog_option")}
onFocus={onFocus}
inputRef={ref}
ref={ref}
tabIndex={-1}
aria-selected={isActive}
role="option"
Expand Down
43 changes: 24 additions & 19 deletions src/components/views/elements/AccessibleButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
limitations under the License.
*/

import React, { HTMLAttributes, InputHTMLAttributes } from "react";
import React, { forwardRef, FunctionComponent, HTMLAttributes, InputHTMLAttributes, Ref } from "react";
import classnames from "classnames";

import { getKeyBindingsManager } from "../../../KeyBindingsManager";
Expand Down Expand Up @@ -66,7 +66,6 @@ type DynamicElementProps<T extends keyof JSX.IntrinsicElements> = Partial<
* Extends props accepted by the underlying element specified using the `element` prop.
*/
type Props<T extends keyof JSX.IntrinsicElements> = DynamicHtmlElementProps<T> & {
inputRef?: React.Ref<Element>;
/**
* The base element type. "div" by default.
*/
Expand Down Expand Up @@ -101,22 +100,26 @@ interface RenderedElementProps extends React.InputHTMLAttributes<Element> {
* as a button. Identifies the element as a button, setting proper tab
* indexing and keyboard activation behavior.
*
* If a ref is passed, it will be forwarded to the rendered element as specified using the `element` prop.
*
* @param {Object} props react element properties
* @returns {Object} rendered react
*/
export default function AccessibleButton<T extends keyof JSX.IntrinsicElements>({
element = "div" as T,
onClick,
children,
kind,
disabled,
inputRef,
className,
onKeyDown,
onKeyUp,
triggerOnMouseDown,
...restProps
}: Props<T>): JSX.Element {
const AccessibleButton = forwardRef(function <T extends keyof JSX.IntrinsicElements>(
{
element = "div" as T,
onClick,
children,
kind,
disabled,
className,
onKeyDown,
onKeyUp,
triggerOnMouseDown,
...restProps
}: Props<T>,
ref: Ref<HTMLElement>,
): JSX.Element {
const newProps: RenderedElementProps = restProps;
if (disabled) {
newProps["aria-disabled"] = true;
Expand Down Expand Up @@ -170,7 +173,7 @@ export default function AccessibleButton<T extends keyof JSX.IntrinsicElements>(
}

// Pass through the ref - used for keyboard shortcut access to some buttons
newProps.ref = inputRef;
newProps.ref = ref;

newProps.className = classnames("mx_AccessibleButton", className, {
mx_AccessibleButton_hasKind: kind,
Expand All @@ -180,11 +183,13 @@ export default function AccessibleButton<T extends keyof JSX.IntrinsicElements>(

// React.createElement expects InputHTMLAttributes
return React.createElement(element, newProps, children);
}
});

AccessibleButton.defaultProps = {
// Type assertion required due to forwardRef type workaround in react.d.ts
(AccessibleButton as FunctionComponent).defaultProps = {
role: "button",
tabIndex: 0,
};
(AccessibleButton as FunctionComponent).displayName = "AccessibleButton";

AccessibleButton.displayName = "AccessibleButton";
export default AccessibleButton;
21 changes: 8 additions & 13 deletions src/components/views/elements/AccessibleTooltipButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ See the License for the specific language governing permissions and
limitations under the License.
*/

import React, { SyntheticEvent, FocusEvent, useEffect, useState } from "react";
import React, { SyntheticEvent, FocusEvent, forwardRef, useEffect, Ref, useState, ComponentProps } from "react";

import AccessibleButton from "./AccessibleButton";
import Tooltip, { Alignment } from "./Tooltip";
Expand All @@ -25,7 +25,7 @@ import Tooltip, { Alignment } from "./Tooltip";
*
* Extends that of {@link AccessibleButton}.
*/
type Props<T extends keyof JSX.IntrinsicElements> = React.ComponentProps<typeof AccessibleButton<T>> & {
type Props<T extends keyof JSX.IntrinsicElements> = ComponentProps<typeof AccessibleButton<T>> & {
/**
* Title to show in the tooltip and use as aria-label
*/
Expand Down Expand Up @@ -60,16 +60,10 @@ type Props<T extends keyof JSX.IntrinsicElements> = React.ComponentProps<typeof
onHideTooltip?(ev: SyntheticEvent): void;
};

function AccessibleTooltipButton<T extends keyof JSX.IntrinsicElements>({
title,
tooltip,
children,
forceHide,
alignment,
onHideTooltip,
tooltipClassName,
...props
}: Props<T>): JSX.Element {
const AccessibleTooltipButton = forwardRef(function <T extends keyof JSX.IntrinsicElements>(
{ title, tooltip, children, forceHide, alignment, onHideTooltip, tooltipClassName, ...props }: Props<T>,
ref: Ref<HTMLElement>,
) {
const [hover, setHover] = useState(false);

useEffect(() => {
Expand Down Expand Up @@ -108,12 +102,13 @@ function AccessibleTooltipButton<T extends keyof JSX.IntrinsicElements>({
onFocus={onFocus}
onBlur={hideTooltip}
aria-label={title || props["aria-label"]}
ref={ref}
>
{children}
{props.label}
{(tooltip || title) && tip}
</AccessibleButton>
);
}
});

export default AccessibleTooltipButton;
2 changes: 1 addition & 1 deletion src/components/views/elements/AppTile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -789,7 +789,7 @@ export default class AppTile extends React.Component<IProps, IState> {
className="mx_AppTileMenuBar_widgets_button"
label={_t("common|options")}
isExpanded={this.state.menuDisplayed}
inputRef={this.contextMenuButton}
ref={this.contextMenuButton}
onClick={this.onContextMenuClick}
>
<MenuIcon className="mx_Icon mx_Icon_12" />
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/Dropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -392,7 +392,7 @@ export default class Dropdown extends React.Component<DropdownProps, IState> {
aria-haspopup="listbox"
aria-expanded={this.state.expanded}
disabled={this.props.disabled}
inputRef={this.buttonRef}
ref={this.buttonRef}
aria-label={this.props.label}
aria-describedby={`${this.props.id}_value`}
aria-owns={`${this.props.id}_input`}
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/ImageView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -506,7 +506,7 @@ export default class ImageView extends React.Component<IProps, IState> {
className="mx_ImageView_button mx_ImageView_button_more"
title={_t("common|options")}
onClick={this.onOpenContextMenu}
inputRef={this.contextMenuButton}
ref={this.contextMenuButton}
isExpanded={this.state.contextMenuDisplayed}
/>
);
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/elements/PollCreateDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,7 @@ export default class PollCreateDialog extends ScrollableBaseModal<IProps, IState
disabled={this.state.busy || this.state.options.length >= MAX_OPTIONS}
kind="secondary"
className="mx_PollCreateDialog_addOption"
inputRef={this.addOptionRef}
ref={this.addOptionRef}
>
{_t("poll|options_add_button")}
</AccessibleButton>
Expand Down
2 changes: 1 addition & 1 deletion src/components/views/messages/CallEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {
import defaultDispatcher from "../../../dispatcher/dispatcher";
import type { ViewRoomPayload } from "../../../dispatcher/payloads/ViewRoomPayload";
import { Action } from "../../../dispatcher/actions";
import { AccessibleButtonKind, ButtonEvent } from "../elements/AccessibleButton";
import type { AccessibleButtonKind, ButtonEvent } from "../elements/AccessibleButton";
import MemberAvatar from "../avatars/MemberAvatar";
import { LiveContentSummary, LiveContentType } from "../rooms/LiveContentSummary";
import FacePile from "../elements/FacePile";
Expand Down
Loading

0 comments on commit f632e21

Please sign in to comment.