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

Code example that introduces TypeError. #12210

Closed
2 tasks done
LAITONEN opened this issue Jul 20, 2018 · 3 comments
Closed
2 tasks done

Code example that introduces TypeError. #12210

LAITONEN opened this issue Jul 20, 2018 · 3 comments
Labels
component: menu This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. typescript

Comments

@LAITONEN
Copy link
Contributor

  • This is a v1.x issue.
  • I have searched the issues of this repository and believe that this is not a duplicate.

The problem is here:

  1. Link: https://material-ui.com/demos/menus/#simple-menu
  2. Open the Source Code of the first example in Simple Menu section.

I have a TypeScript project, where I decided to use Menu Component. After copy-pasting the code from the documentation, I got this TypeError for Menu Component:

Type 'null' is not assignable to type 'HTMLElement | undefined'.

Which means that the value of anchorEl can not be null, whereas the documentation implies that it could. Setting the value of anchorEl to undefined solves the problem.

Your Environment

Tech Version
Material-UI v1.0.0-beta.26
Typescript 2.8.3
VS Code 1.25.1
@oliviertassinari oliviertassinari added good first issue Great for first contributions. Enable to learn the contribution process. component: menu This is the name of the generic UI component, not the React module! typescript labels Jul 20, 2018
@oliviertassinari
Copy link
Member

@LAITONEN We have been fixing the very same issue a few days ago for the Popper component. We need to change this line
https://github.com/mui-org/material-ui/blob/ba2ba252c2389cdde4e44d0ccb149c6ba466b349/packages/material-ui/src/Menu/Menu.d.ts#L11
as it was done here: #12161 (comment)
Do you want to work on it?

@LAITONEN
Copy link
Contributor Author

@oliviertassinari I could. You think it is the typing that should be corrected, not the default value of this.state.anchorEl in the example (from null to undefined)?

@oliviertassinari
Copy link
Member

oliviertassinari commented Jul 20, 2018

@LAITONEN I'm suggesting that we change material-ui/packages/material-ui/src/Menu/Menu.d.ts as we did for the Popper: #12161 (comment):

-  anchorEl?: HTMLElement | ((element: HTMLElement) => HTMLElement);
+  anchorEl?: null | HTMLElement | ((element: HTMLElement) => HTMLElement);

LAITONEN added a commit to LAITONEN/material-ui that referenced this issue Jul 23, 2018
oliviertassinari pushed a commit that referenced this issue Jul 23, 2018
* Added null as acceptable value of anchorEl

#12210 (comment)

* let's merge
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: menu This is the name of the generic UI component, not the React module! good first issue Great for first contributions. Enable to learn the contribution process. typescript
Projects
None yet
Development

No branches or pull requests

2 participants