Skip to content
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.

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

[ClickAwayListener] Fix children and onClickAway types #24565

Merged
merged 12 commits into from
Jan 27, 2021
2 changes: 1 addition & 1 deletion docs/pages/api-docs/click-away-listener.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"name": "ClickAwayListener",
"styles": { "classes": [], "globalClasses": {}, "name": null },
"spread": false,
"filename": "/packages/material-ui/src/ClickAwayListener/ClickAwayListener.js",
"filename": "/packages/material-ui/src/ClickAwayListener/ClickAwayListener.tsx",
"inheritance": null,
"demos": "<ul><li><a href=\"/components/click-away-listener/\">Click Away Listener</a></li>\n<li><a href=\"/components/menus/\">Menus</a></li></ul>",
"styledComponent": true,
Expand Down
18 changes: 16 additions & 2 deletions docs/scripts/buildApi.ts
Original file line number Diff line number Diff line change
Expand Up @@ -505,7 +505,21 @@ async function annotateComponentDefinition(context: {
if (babel.types.isIdentifier(babelPath.node.declaration)) {
const bindingId = babelPath.node.declaration.name;
const binding = babelPath.scope.bindings[bindingId];
node = binding.path.parentPath.node;

// The JSDOC MUST be located at the declaration
if (babel.types.isFunctionDeclaration(binding.path.node)) {
// For function declarations the binding is equal to the declaration
// /**
// */
// function Component() {}
node = binding.path.node;
} else {
// For variable declarations the binding points to the declarator.
// /**
// */
// const Component = () => {}
node = binding.path.parentPath.node;
}
}
}

Expand Down Expand Up @@ -906,7 +920,7 @@ async function parseComponentSource(
// Ignore what we might have generated in `annotateComponentDefinition`
const annotatedDescriptionMatch = fullDescription.match(/(Demos|API):\r?\n\r?\n/);
if (annotatedDescriptionMatch !== null) {
reactAPI.description = fullDescription.slice(0, annotatedDescriptionMatch.index);
reactAPI.description = fullDescription.slice(0, annotatedDescriptionMatch.index).trim();
}

return reactAPI;
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/button-group/SplitButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ export default function SplitButton() {
setOpen((prevOpen) => !prevOpen);
};

const handleClose = (event: React.MouseEvent<Document, MouseEvent>) => {
const handleClose = (event: Event) => {
if (
anchorRef.current &&
anchorRef.current.contains(event.target as HTMLElement)
Expand Down
2 changes: 1 addition & 1 deletion docs/src/pages/components/menus/MenuListComposition.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ export default function MenuListComposition() {
setOpen((prevOpen) => !prevOpen);
};

const handleClose = (event: React.MouseEvent<EventTarget>) => {
const handleClose = (event: Event | React.SyntheticEvent) => {
if (
anchorRef.current &&
anchorRef.current.contains(event.target as HTMLElement)
Expand Down
2 changes: 1 addition & 1 deletion packages/material-ui-utils/src/ownerDocument.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
export default function ownerDocument(node: Node | undefined): Document {
export default function ownerDocument(node: Node | null | undefined): Document {
return (node && node.ownerDocument) || document;
}
43 changes: 0 additions & 43 deletions packages/material-ui/src/ClickAwayListener/ClickAwayListener.d.ts

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -5,22 +5,64 @@ import ownerDocument from '../utils/ownerDocument';
import useForkRef from '../utils/useForkRef';
import useEventCallback from '../utils/useEventCallback';

function mapEventPropToEvent(eventProp) {
return eventProp.substring(2).toLowerCase();
// TODO: return `EventHandlerName extends `on${infer EventName}` ? Lowercase<EventName> : never` once generatePropTypes runs with TS 4.1
function mapEventPropToEvent(
eventProp: ClickAwayMouseEventHandler | ClickAwayTouchEventHandler,
): 'click' | 'mousedown' | 'mouseup' | 'touchstart' | 'touchend' {
return eventProp.substring(2).toLowerCase() as any;
}

function clickedRootScrollbar(event, doc) {
function clickedRootScrollbar(event: MouseEvent, doc: Document) {
return (
doc.documentElement.clientWidth < event.clientX ||
doc.documentElement.clientHeight < event.clientY
);
}

type ClickAwayMouseEventHandler = 'onClick' | 'onMouseDown' | 'onMouseUp';
type ClickAwayTouchEventHandler = 'onTouchStart' | 'onTouchEnd';

export interface ClickAwayListenerProps {
/**
* The wrapped element.
*/
children: React.ReactElement;
/**
* If `true`, the React tree is ignored and only the DOM tree is considered.
* This prop changes how portaled elements are handled.
* @default false
*/
disableReactTree?: boolean;
/**
* The mouse event to listen to. You can disable the listener by providing `false`.
* @default 'onClick'
*/
mouseEvent?: ClickAwayMouseEventHandler | false;
/**
* Callback fired when a "click away" event is detected.
*/
onClickAway: (event: MouseEvent | TouchEvent) => void;
/**
* The touch event to listen to. You can disable the listener by providing `false`.
* @default 'onTouchEnd'
*/
touchEvent?: ClickAwayTouchEventHandler | false;
}

/**
* Listen for click events that occur somewhere in the document, outside of the element itself.
* For instance, if you need to hide a menu when people click anywhere else on your page.
*
* Demos:
*
* - [Click Away Listener](https://material-ui.com/components/click-away-listener/)
* - [Menus](https://material-ui.com/components/menus/)
*
* API:
*
* - [ClickAwayListener API](https://material-ui.com/api/click-away-listener/)
*/
function ClickAwayListener(props) {
function ClickAwayListener(props: ClickAwayListenerProps): JSX.Element {
const {
children,
disableReactTree = false,
Expand All @@ -29,7 +71,7 @@ function ClickAwayListener(props) {
touchEvent = 'onTouchEnd',
} = props;
const movedRef = React.useRef(false);
const nodeRef = React.useRef(null);
const nodeRef = React.useRef<Element>(null);
const activatedRef = React.useRef(false);
const syntheticEventRef = React.useRef(false);

Expand All @@ -44,15 +86,19 @@ function ClickAwayListener(props) {
};
}, []);

const handleRef = useForkRef(children.ref, nodeRef);
const handleRef = useForkRef(
// @ts-expect-error TODO upstream fix
children.ref,
nodeRef,
);

// The handler doesn't take event.defaultPrevented into account:
//
// event.preventDefault() is meant to stop default behaviors like
// clicking a checkbox to check it, hitting a button to submit a form,
// and hitting left arrow to move the cursor in a text input etc.
// Only special HTML elements have these default behaviors.
const handleClickAway = useEventCallback((event) => {
const handleClickAway = useEventCallback((event: MouseEvent | TouchEvent) => {
// Given developers can stop the propagation of the synthetic event,
// we can only be confident with a positive value.
const insideReactTree = syntheticEventRef.current;
Expand All @@ -63,7 +109,11 @@ function ClickAwayListener(props) {
// 1. IE11 support, which trigger the handleClickAway even after the unbind
// 2. The child might render null.
// 3. Behave like a blur listener.
if (!activatedRef.current || !nodeRef.current || clickedRootScrollbar(event, doc)) {
if (
!activatedRef.current ||
!nodeRef.current ||
('clientX' in event && clickedRootScrollbar(event, doc))
Copy link
Member Author

Choose a reason for hiding this comment

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

We used to access event.clientX on touch events which don't implement that property. So we would always end up evaluating doc.documentElement.clientWidth < undefined || doc.documentElement.clientHeight < undefined. This always evaluates to false so we can bail out early by checking if we have a MouseEvent.

In the end we're forcing less layout on touch events (no access to element.clientWidth) but check property existence more often. Seems like a good tradeoff considering forcing layout is much more expensive than checking property existence and touch events are usually dispatched on weaker devices e.g. smartphones.

) {
return;
}

Expand All @@ -80,7 +130,14 @@ function ClickAwayListener(props) {
insideDOM = event.composedPath().indexOf(nodeRef.current) > -1;
} else {
insideDOM =
!doc.documentElement.contains(event.target) || nodeRef.current.contains(event.target);
!doc.documentElement.contains(
// @ts-expect-error returns `false` as intended when not dispatched from a Node
event.target,
) ||
nodeRef.current.contains(
// @ts-expect-error returns `false` as intended when not dispatched from a Node
event.target,
);
}

if (!insideDOM && (disableReactTree || !insideReactTree)) {
Expand All @@ -89,7 +146,7 @@ function ClickAwayListener(props) {
});

// Keep track of mouse/touch events that bubbled up through the portal.
const createHandleSynthetic = (handlerName) => (event) => {
const createHandleSynthetic = (handlerName: string) => (event: React.SyntheticEvent) => {
syntheticEventRef.current = true;

const childrenPropsHandler = children.props[handlerName];
Expand All @@ -98,7 +155,10 @@ function ClickAwayListener(props) {
}
};

const childrenProps = { ref: handleRef };
const childrenProps: { ref: React.Ref<Element> } & Pick<
React.DOMAttributes<Element>,
ClickAwayMouseEventHandler | ClickAwayTouchEventHandler
> = { ref: handleRef };

if (touchEvent !== false) {
childrenProps[touchEvent] = createHandleSynthetic(touchEvent);
Expand Down Expand Up @@ -150,7 +210,7 @@ function ClickAwayListener(props) {
ClickAwayListener.propTypes = {
// ----------------------------- Warning --------------------------------
// | These PropTypes are generated from the TypeScript type definitions |
// | To update them edit the d.ts file and run "yarn proptypes" |
// | To update them edit TypeScript types and run "yarn proptypes" |
// ----------------------------------------------------------------------
/**
* The wrapped element.
Expand Down Expand Up @@ -180,7 +240,7 @@ ClickAwayListener.propTypes = {

if (process.env.NODE_ENV !== 'production') {
// eslint-disable-next-line
ClickAwayListener['propTypes' + ''] = exactProp(ClickAwayListener.propTypes);
(ClickAwayListener as any)['propTypes' + ''] = exactProp(ClickAwayListener.propTypes);
}

export default ClickAwayListener;
1 change: 0 additions & 1 deletion packages/material-ui/src/ClickAwayListener/index.js

This file was deleted.

4 changes: 2 additions & 2 deletions packages/material-ui/tsconfig.build.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"outDir": "build",
"rootDir": "./src"
},
"include": ["src/**/*.ts"],
"exclude": ["src/**/*.spec.ts", "src/**/*.test.ts"],
"include": ["./src/**/*.ts*"],
"exclude": ["src/**/*.spec.ts*", "src/**/*.test.ts*"],
"references": [{ "path": "../material-ui-unstyled/tsconfig.build.json" }]
}
2 changes: 2 additions & 0 deletions scripts/generateProptypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,6 +350,8 @@ async function run(argv: HandlerArgv) {
.filter((filePath) => {
return filePattern.test(filePath);
});
// May not be able to understand all files due to mismatch in TS versions.
// Check `programm.getSyntacticDiagnostics()` if referenced files could not be compiled.
const program = ttp.createTSProgram(files, tsconfig);

const promises = files.map<Promise<GenerateResult>>(async (tsFile) => {
Expand Down