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

[Container] Move to the core #15062

Merged
merged 1 commit into from
Mar 27, 2019

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Mar 26, 2019

It's related to #14519. We will add more layout components in the near future. This container component targets simple pages. We need new components for the dashboard use case.

Breaking change

-import Container from '@material-ui/lab/Container';
+import Container from '@material-ui/core/Container';

@mui-pr-bot
Copy link

@material-ui/core: parsed: +0.45% , gzip: +0.01%

Details of bundle changes.

Comparing: 08eb5da...16d4b36

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.45% 🔺 +0.01% 🔺 349,795 351,379 90,100 90,105
@material-ui/core/Paper 0.00% -0.02% 67,869 67,869 19,832 19,829
@material-ui/core/Paper.esm 0.00% 0.00% 60,201 60,201 18,572 18,572
@material-ui/core/Popper 0.00% +0.01% 🔺 30,460 30,460 10,528 10,529
@material-ui/core/styles/createMuiTheme 0.00% +0.02% 🔺 17,384 17,384 5,726 5,727
@material-ui/core/useMediaQuery 0.00% 0.00% 2,469 2,469 1,043 1,043
@material-ui/lab -0.02% 0.00% 148,690 148,656 43,684 43,686
@material-ui/styles 0.00% 0.00% 53,102 53,102 15,444 15,444
@material-ui/system 0.00% +0.04% 🔺 17,136 17,136 4,527 4,529
Button 0.00% 0.00% 88,356 88,356 26,177 26,178
Modal 0.00% 0.00% 82,460 82,461 24,670 24,671
colorManipulator 0.00% 0.00% 3,232 3,232 1,299 1,299
docs.landing 0.00% 0.00% 49,958 49,958 10,837 10,837
docs.main 0.00% +0.02% 🔺 646,389 646,391 200,783 200,820
packages/material-ui/build/umd/material-ui.production.min.js +0.43% 🔺 +0.37% 🔺 298,354 299,628 82,635 82,941

Generated by 🚫 dangerJS against 16d4b36

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 26, 2019

I have a few more pages I want to rename. Should we do it in a single batch? Maybe it will be simpler for i18n. cc @mbrookes.

@mbrookes
Copy link
Member

If anything it may be easier to phase them. Each one will need to be checked for each language to ensure that "Translation Memory" has reapplied existing translations.

Let's benchmark with this one (assuming it has some translations already?).

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Mar 27, 2019

Yes, it should already have a few translations.

@oliviertassinari oliviertassinari merged commit be889d9 into mui:next Mar 27, 2019
@oliviertassinari oliviertassinari deleted the move-layout-core branch March 27, 2019 09:03
@mbrookes
Copy link
Member

It seem something went wrong, but it appears to be a Crowdin problem:

image

I'll take it up with support.

Aside:
What do you think about naming the "Layout" page "Container"? It is confusing having a page called layout in the layout section, especially when it is not generally about layout, but specifically about the use of Container. It also makes the Container documentation harder to find:

image

Also, as it stands, on this Layout page, CssBaseline and Container have the same heading level. Except for the link, it makes it seem that this is the documentation for CssBaseline, rather than a hint to use it along with Container. A block quote might work.

@oliviertassinari
Copy link
Member Author

What do you think about naming the "Layout" page "Container"?

I would like to add more layout components. We need something to assemble the app-bar and the drawer/side menu.

@oliviertassinari
Copy link
Member Author

Do you think that we should flatten it? Have one documentation page for each component or keep the current solution?

  • /layout/Container
  • /layout/Side
  • /layout/Header
  • /etc.

@mbrookes
Copy link
Member

I would prefer that we have one page for each component, as it's easier to navigate in a couple of ways:

  1. It's easier to find the page you need if it is named for the component it describes, and is consistent with the way the majority of components are documented (selection controls being one exception).

  2. It's easier not having to scroll to the relevant section.

I think there's a further discussion to be had regarding the overall structure of the docs. With the addition so many new new packages, it's getting harder to navigate, but let's save that for its own issue. We can worry about it after v4, or beyond.

@oliviertassinari
Copy link
Member Author

Good for me. We can change the structure.

@mbrookes
Copy link
Member

I can understand the logic behind grouping them, as they go together as a set. Perhaps a future 3rd level in the page / navigation hierarchy would help? (It's one of the things I wanted to discuss regarding the structure of the docs.)

We could defer changing this until that's decided. If so, then perhaps /layout/page-layout would be clearer?

@oliviertassinari
Copy link
Member Author

I would like to avoid 3rd level as much as possible. I'm happy to flatten it, really :).

@mbrookes
Copy link
Member

mbrookes commented Apr 2, 2019

#15062 (comment)

Circling back to confirm that phasing it will be easier. I had to tidy up each translation of container.md.

@oliviertassinari
Copy link
Member Author

Thanks for following up on this.

@oliviertassinari oliviertassinari added the component: Container The React component label Nov 28, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants