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

[docs] Base ClickAwayListener style revisions and final review #32156

Merged
merged 12 commits into from Apr 29, 2022

Conversation

samuelsycamore
Copy link
Member

This PR reviews the ClickAwayListener doc. I've done some style editing, and also elaborated on parts of the text that were a bit sparse. I have a couple questions about the content that I will ask as comments on the doc.

@samuelsycamore samuelsycamore added docs Improvements or additions to the documentation component: ClickAwayListener The React component package: base-ui Specific to @mui/base labels Apr 7, 2022
@mui-bot
Copy link

mui-bot commented Apr 7, 2022

Details of bundle changes

@material-ui/core: parsed: +Infinity% , gzip: +Infinity%
@material-ui/lab: parsed: +Infinity% , gzip: +Infinity%
@material-ui/styles: parsed: +Infinity% , gzip: +Infinity%
@material-ui/private-theming: parsed: +Infinity% , gzip: +Infinity%
@material-ui/system: parsed: +Infinity% , gzip: +Infinity%
@material-ui/unstyled: parsed: +Infinity% , gzip: +Infinity%
@material-ui/utils: parsed: +Infinity% , gzip: +Infinity%
@mui/material-next: parsed: +Infinity% , gzip: +Infinity%
@mui/joy: parsed: +Infinity% , gzip: +Infinity%

Generated by 🚫 dangerJS against 3d29e61

@samuelsycamore samuelsycamore requested a review from a team April 7, 2022 01:12
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 8, 2022
Copy link
Member

@michaldudak michaldudak left a comment

Choose a reason for hiding this comment

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

I found two minor issues. Besides that it looks good!

samuelsycamore and others added 3 commits April 27, 2022 10:09
…ner.md

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
…ner.md

Co-authored-by: Michał Dudak <michal.dudak@gmail.com>
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 27, 2022
@samuelsycamore samuelsycamore merged commit d990295 into mui:master Apr 29, 2022
@samuelsycamore samuelsycamore deleted the base-cal-style-review branch April 29, 2022 18:20
@@ -1,58 +1,64 @@
---
product: base
title: Detect click outside React component
Copy link
Member

Choose a reason for hiding this comment

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

This title was on purpose for SEO. See https://stackoverflow.com/questions/32553158/detect-click-outside-react-component for why. Sure it might not be for the title but we still need to rank for this search intent:

Screenshot 2022-04-30 at 17 53 38

I have updated https://stackoverflow.com/a/59913572/2801714 to point to the latest URL.

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 can revert this title in the final pass of the Base docs. I'll make a note of it.


```tsx
<ClickAwayListener>
<div role="presentation">
<h1>non-interactive heading</h1>
</div>
</ClickAwayListern>
</ClickAwayListener>
Copy link
Member

Choose a reason for hiding this comment

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

As a general note, it can be great to search for other instances of the problem in the codebase. It's not rare to have the problem not isolated e.g. #33813 😁.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component: ClickAwayListener The React component docs Improvements or additions to the documentation package: base-ui Specific to @mui/base
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants