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

Embrace DOM-like structure #27

Closed
piranha opened this issue Nov 10, 2014 · 12 comments
Closed

Embrace DOM-like structure #27

piranha opened this issue Nov 10, 2014 · 12 comments
Assignees
Labels
new feature New feature or request

Comments

@piranha
Copy link

piranha commented Nov 10, 2014

Right now subcomponents are passed as a list of special objects to some specific prop, like there:

menuItems = [
  { route: 'get-started', text: 'Get Started' },
  { route: 'css-framework', text: 'CSS Framework' },
  { route: 'components', text: 'Components' },
  { type: MenuItem.Types.SUBHEADER, text: 'Resources' },
  { 
     type: MenuItem.Types.LINK, 
     payload: 'https://github.com/callemall/material-ui', 
     text: 'GitHub' 
  },
];

//Docked Left Nav
<LeftNav menuItems={menuItems} />

I'd argue it would be much more composable and extensible to reworking API to have something like this:

<LeftNav>
  <MenuItem><Route to="get-started">Get Started</Route></MenuItem>
  <Subheader>Resources</Subheader>
  <MenuItem><a href="...">GitHub</a></MenuItem>
</LeftNav>

The same for all menus.

@grechut
Copy link

grechut commented Nov 10, 2014

👍

2 similar comments
@felixakiragreen
Copy link

👍

@mcwhittemore
Copy link

+1

@hai-cea
Copy link
Member

hai-cea commented Nov 11, 2014

Yes, I think we'd definitely like to go in this direction. We'll work this into the next release or 2.

@hai-cea hai-cea self-assigned this Nov 11, 2014
@Farhan77
Copy link

H

@kylefinley
Copy link
Contributor

Maybe you could keep both. For menus and navigation, where the structure is well known, I think it makes sense to make it more of a configuration. If I want to quickly implement a menu I don't want to import/remember all of the sub menu components.

Wouldn't a simple if statement allow for overrides, though. Something like:

if (this.props.children) {
 return this.props.children;
} else {
  // Build it using the config
}

@mcwhittemore
Copy link

@kylefinley are you looking to have the config so you don't have to recreate the same jsx over and over again? If so, how are you doing this with the config? Can you just replace that with JSX dom structures?

@kylefinley
Copy link
Contributor

@mcwhittemore,

Yes, I see your point.

I'm new to React, so perhaps my suggestion is not idiomatic. But the thing I like about the current implementation, and React for that matter, is that it feels more like JavaScript than HTML. Instead of using JSX, or even nested React functions; Material-UI simply uses JavaScript data structure (Objects and Arrays) to define the structure, E.g. Menus

At the end of the day it's just personal preference. I just though that the use of Objects and Arrays to define the structure was novel. Obviously it only works if you are able to optimize for the 95% usecase and allow overriding with JSX dom for the remaining 5%.

@mcwhittemore
Copy link

@kylefinley I feel you. One of the things that using react also does though is let your templates and your js merge so that your typical html/css devs work on end product and not just mocks.

@arianvp
Copy link

arianvp commented Jan 21, 2015

Has there be any updates in this regard? I'm very interested in this.

skv-headless added a commit to skv-headless/material-ui that referenced this issue Mar 3, 2015
@eliquious
Copy link

+1

@hai-cea hai-cea mentioned this issue Jun 27, 2015
@hai-cea hai-cea added this to the Composable Components milestone Jul 13, 2015
@yongxu
Copy link
Contributor

yongxu commented Aug 2, 2015

#1319
I made this that does exactly what you are expecting!

@alitaheri alitaheri added Enhancement new feature New feature or request and removed Status: Working labels Dec 8, 2015
@alitaheri alitaheri modified the milestone: Composable Components Dec 23, 2015
brianfeister pushed a commit to brianfeister/material-ui that referenced this issue May 1, 2017
…ection contains ALL libraries that are required for the server runtime. The "devDependencies" section contains the libraries required for building the project. Fixes mui#27
@michaldudak michaldudak moved this to Done 🎉 in Base UI Alpha Dec 17, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
new feature New feature or request
Projects
None yet
Development

No branches or pull requests