[docs] Base Portal style revisions and final review #32157

mapache-salvaje · 2022-04-07T01:34:25Z

This PR reviews the Portal doc. The content is sparse but that makes sense because there isn't much to say. 🤷‍♂️ This one is pretty deep in the weeds, but I think it would be worthwhile to elaborate on its use cases.

Edit: the second draft explains more about portals and their usage.

https://deploy-preview-32157--material-ui.netlify.app/base/react-portal/

docs/data/base/components/portal/portal.md

mui-bot · 2022-04-07T01:36:31Z

No bundle size changes

Generated by 🚫 dangerJS against 8973451

docs/data/base/components/portal/portal.md

Co-authored-by: Michał Dudak <[email protected]>

mnajdova · 2022-05-04T15:26:40Z

docs/data/base/components/portal/portal.md

+## Basic usage
+
+Normally, children of a component are rendered within that component's DOM tree.
+But sometimes it's necessary to mount a child to a different location in the DOM.


Why? We should give here example/explanation

I did a little bit of rearranging to elaborate on this. Let me know what you think!

oliviertassinari · 2022-05-04T15:29:16Z

docs/data/base/components/portal/portal.md

@@ -8,18 +8,28 @@ packageName: '@mui/base'

 # Portal

-<p class="description">The portal component renders its children into a new "subtree" outside of the current DOM hierarchy.</p>
+<p class="description">The <code>Portal</code> component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>


This doesn't work with Google:

<meta name="description" content="The <code>Portal</code> component lets you render its children into a DOM node that exists outside of its own DOM hierarchy."/>

It would show like this, no go

I think that the following would be better, not talking about React, but the pattern in general and without a disturbing style change:

Suggested change

The <code>Portal</code> component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.

The portal component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.

Ah shoot, this is something we'll have to fix across the board with the Base docs.

It's possible to duplicate the description, having one specific for SEO, but personally, I would encourage we make a change so that the docs throw an error when <code> or any HTML element is used in the description (easy change)

Sounds good. Two descriptions could get messy pretty quickly. I think we should still capitalize it here at least, since we're talking about our specific component rather than the general concept.

oliviertassinari · 2022-05-04T15:30:09Z

docs/data/base/components/portal/portal.md


-The children of the portal component will be appended to the `container` specified.
-The component is used internally by the [`Modal`](/material-ui/react-modal/) and [`Popper`](/material-ui/react-popper/) components.
+`Portal` is a utility component built around [React's `createPortal()` API](https://reactjs.org/docs/portals.html).


It think that we can keep the content more "lightweight" (easier to read, less style change interruption) with:

Suggested change

`Portal` is a utility component built around [React's `createPortal()` API](https://reactjs.org/docs/portals.html).

Portal is a utility component built around [React's `createPortal()` API](https://reactjs.org/docs/portals.html).

We don't need to really React in this case?

I think it's helpful here to differentiate between Portal (the component) vs. any generic portal generated by the createPortal() API.

The distinction already seems clear from "Portal is a utility component".

Looking at the Material UI docs, we don't seem to be consistent in the use of backticks for component names. It does seem the be the predominant style, but it isn't necessarily better. The capitalisation seems sufficient, along with an indefinite article: "a portal" vs. "Portal"

Looking at the Material UI docs, we don't seem to be consistent in the use of backticks for component names. It does seem the be the predominant style, but it isn't necessarily better. The capitalisation seems sufficient, along with an indefinite article: "a portal" vs. "Portal"

I thought we had established a house rule for this in the Style guide, but I'm not seeing anything about it now that I'm looking through it. Must have been deep in some PR comments somewhere.

In the Base docs, I've been trying to stick to a consistent format of backticks and capitalization anytime I'm referring to the component specifically, in the interest of being as clear and precise as possible.

In any case, this is a question that applies to all of the Base docs, so I propose that we leave it as is for now, and continue this conversation about formatting to my next PR which will be a final pass covering all of these pages.

docs/data/base/components/portal/portal.md

Co-authored-by: Matt <[email protected]>

danilo-leal

Nice!

mbrookes · 2022-05-07T13:48:19Z

docs/data/base/components/portal/portal.md

@@ -8,18 +8,33 @@ packageName: '@mui/base'

 # Portal

-<p class="description">The portal component renders its children into a new "subtree" outside of the current DOM hierarchy.</p>
+<p class="description">The Portal component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>


For most other components we describe the general use, so:

Suggested change

The Portal component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.

A portal lets you render its children into a DOM node that exists outside of its own DOM hierarchy.

I've been doing a mix of both across the Base docs—sometimes one makes more sense than the other. See: Button vs Click-away listener.

In the case of the Portal, I think it makes sense either way.

mbrookes · 2022-05-07T13:56:21Z

docs/data/base/components/portal/portal.md

+See [this GitHub issue](https://github.com/facebook/react/issues/13097) for details.
+:::
+
+The `Portal` component cannot be used to render child elements on the server—client-side hydration is necessary.


Suggested change

The `Portal` component cannot be used to render child elements on the server—client-side hydration is necessary.

Portal can't be used to render child elements on the server—client-side hydration is necessary.

Same as this comment, I propose we return to this question in the next PR that I will open for a final pass across all Base docs at once.

mbrookes · 2022-05-07T14:00:20Z

Once finalised is the content going to be copied in the Material UI docs?

mapache-salvaje · 2022-05-09T16:50:05Z

Once finalised is the content going to be copied in the Material UI docs?

Great question! There is some overlap between the Base and Material UI docs, so when I do move on to revamp the latter in the next couple months, I will most likely start with these pages. Unless you think we should move this content over ASAP.

michaldudak

👍

nuarhu · 2022-07-20T13:24:13Z

Looking for an example on how to integrate a portal in two separate child components in the component tree, I stumbled over this issue here. I would really wish there would be a more complex example in the MUI documentation that shows how to do that. The current documentation is kindof simple in terms of the container ref being used in a single component.

There might be a basic React solution how to do that but some pointers or hints on this page would be really helpful (for me at least). Once I've found out how to do this, I might be able to contribute but right now I'm still looking for the solution to this problem.

michaldudak · 2022-07-27T11:33:37Z

Could you explain what you mean by "two separate child components in the component tree"?
Would you like to be able to render the same content in two places?

nuarhu · 2022-07-29T08:22:14Z

@michaldudak thanks for your response!

This was my use case:

The app has a toolbar, a drawer with RouterTab navigation, and main content.
The main content changes based on the selection in the sidebar, which triggers a route change.

The toolbar might contain different actions based on the selected route.
The main content and the toolbar are different child components of a base component that contains the Outlet for the routing.

The component with the main content would have the business logic which actions to show in the toolbar.

I've solved it now using state, and will use more Redux more heavily to do that. Which is probably what I should have done from the beginning?

Thanks!
Chantal

style revisions

07402cd

mapache-salvaje added docs Improvements or additions to the documentation component: Portal The React component. package: base-ui Specific to @mui/base labels Apr 7, 2022

mapache-salvaje commented Apr 7, 2022

View reviewed changes

docs/data/base/components/portal/portal.md Outdated Show resolved Hide resolved

mapache-salvaje mentioned this pull request Apr 7, 2022

[docs] Improve the Base documentation #32072

Closed

20 tasks

mapache-salvaje requested a review from a team April 7, 2022 01:47

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 8, 2022

update

884952b

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 25, 2022

second draft, elaborate on what portals are

ca5293b

mapache-salvaje requested review from mnajdova and michaldudak April 25, 2022 20:05

clarify material ui components

7b271bb

mapache-salvaje commented Apr 25, 2022

View reviewed changes

docs/data/base/components/portal/portal.md Show resolved Hide resolved

pass the tests baby

c2ef49f

michaldudak reviewed Apr 29, 2022

View reviewed changes

docs/data/base/components/portal/portal.md Outdated Show resolved Hide resolved

Update docs/data/base/components/portal/portal.md

b914451

Co-authored-by: Michał Dudak <[email protected]>

mapache-salvaje requested a review from michaldudak April 29, 2022 18:06

samuelsycamore added 4 commits April 29, 2022 13:26

prettier

a6f793b

add new callouts

b3eb920

merge master

5d95aa4

update material ui links

4ef6bae

mnajdova reviewed May 4, 2022

View reviewed changes

oliviertassinari reviewed May 4, 2022

View reviewed changes

mbrookes requested changes May 4, 2022

View reviewed changes

docs/data/base/components/portal/portal.md Outdated Show resolved Hide resolved

docs/data/base/components/portal/portal.md Outdated Show resolved Hide resolved

mapache-salvaje and others added 3 commits May 4, 2022 11:35

Update docs/data/base/components/portal/portal.md

1260f89

Co-authored-by: Matt <[email protected]>

Update docs/data/base/components/portal/portal.md

10a35af

Co-authored-by: Matt <[email protected]>

third draft

8973451

mapache-salvaje requested review from oliviertassinari, mnajdova, mbrookes and danilo-leal May 4, 2022 22:36

danilo-leal approved these changes May 6, 2022

View reviewed changes

mbrookes reviewed May 7, 2022

View reviewed changes

michaldudak approved these changes May 9, 2022

View reviewed changes

mapache-salvaje merged commit feddf0e into mui:master May 12, 2022

mapache-salvaje deleted the base-portal-style-review branch April 3, 2024 17:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[docs] Base Portal style revisions and final review #32157

[docs] Base Portal style revisions and final review #32157

mapache-salvaje commented Apr 7, 2022 •

edited by oliviertassinari

Loading

mui-bot commented Apr 7, 2022 •

edited

Loading

mnajdova May 4, 2022

mapache-salvaje May 4, 2022

oliviertassinari May 4, 2022 •

edited

Loading

mapache-salvaje May 4, 2022

oliviertassinari May 4, 2022 •

edited

Loading

mapache-salvaje May 4, 2022

oliviertassinari May 4, 2022

mapache-salvaje May 4, 2022

mbrookes May 7, 2022 •

edited

Loading

mapache-salvaje May 9, 2022

danilo-leal left a comment

mbrookes May 7, 2022

mapache-salvaje May 9, 2022

mbrookes May 7, 2022

mapache-salvaje May 9, 2022

mbrookes commented May 7, 2022

mapache-salvaje commented May 9, 2022

michaldudak left a comment

nuarhu commented Jul 20, 2022 •

edited

Loading

michaldudak commented Jul 27, 2022 •

edited by mbrookes

Loading

nuarhu commented Jul 29, 2022

	<p class="description">The <code>Portal</code> component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>
	<p class="description">The portal component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>

	`Portal` is a utility component built around [React's `createPortal()` API](https://reactjs.org/docs/portals.html).
	Portal is a utility component built around [React's `createPortal()` API](https://reactjs.org/docs/portals.html).

	<p class="description">The Portal component lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>
	<p class="description">A portal lets you render its children into a DOM node that exists outside of its own DOM hierarchy.</p>

	The `Portal` component cannot be used to render child elements on the server—client-side hydration is necessary.
	Portal can't be used to render child elements on the server—client-side hydration is necessary.

[docs] Base Portal style revisions and final review #32157

[docs] Base Portal style revisions and final review #32157

Conversation

mapache-salvaje commented Apr 7, 2022 • edited by oliviertassinari Loading

mui-bot commented Apr 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

oliviertassinari May 4, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes May 7, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

danilo-leal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mbrookes commented May 7, 2022

mapache-salvaje commented May 9, 2022

michaldudak left a comment

Choose a reason for hiding this comment

nuarhu commented Jul 20, 2022 • edited Loading

michaldudak commented Jul 27, 2022 • edited by mbrookes Loading

nuarhu commented Jul 29, 2022

mapache-salvaje commented Apr 7, 2022 •

edited by oliviertassinari

Loading

mui-bot commented Apr 7, 2022 •

edited

Loading

oliviertassinari May 4, 2022 •

edited

Loading

oliviertassinari May 4, 2022 •

edited

Loading

mbrookes May 7, 2022 •

edited

Loading

nuarhu commented Jul 20, 2022 •

edited

Loading

michaldudak commented Jul 27, 2022 •

edited by mbrookes

Loading