mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.38k stars 32.14k forks source link

[LeftNav] rename to SideNav #2697

Closed mbrookes closed 8 years ago

mbrookes commented 8 years ago

Just a thought - given that this component can be opened on the left or right, LeftNav is a bit of a misnomer.

Since component files are being renamed to PascalCase, it would seem like a good opportunity to also rename the component itself.

There will still only be one deprecation warning, and users will be free to import as LeftNav from material-ui/lib/SideNav (or from deprecated material-ui/lib/left-nav) if doing so makes migration easier than renaming instances of LeftNav in use.

If this seems reasonable, I'll make a PR.

newoga commented 8 years ago

I agree with this. :+1: This also goes along with our general strategy to better rename our components to match the spec, as the spec would call this a Side nav.

We may even want to consider deprecating openRight prop to something like openSecondary or something like that to better support RTL theme/languages.

oliviertassinari commented 8 years ago

Noticed that with react-native it's called DrawerLayout https://facebook.github.io/react-native/docs/drawerlayoutandroid.html.

alitaheri commented 8 years ago

It's Stateless Functional Components all over again :disappointed:

newoga commented 8 years ago

Haha, It's also worth mentioning that the spec also calls this a Navigation drawer. Or rather, a type of Side nav is the Navigation drawer. I'm open to any of those names, but still agree with @mbrookes originally sentiment regarding the term "Left" is confusing because it can open from both sides.

@alitaheri How about <StatelessFunctionalSideNav />? :laughing:

alitaheri commented 8 years ago

I agree with @mbrookes. The name is not very intuitive. How about NavigationDrawer with openOpposite prop. In other words the side is automatically changed with rtl. and if they want it to open from the opposite site they set that to true. Thoughts?

@newoga If we decide on that name I will set React on fire :laughing: :laughing:

alitaheri commented 8 years ago

@mbrookes Well in case of RTL language the will have to open from the right side. It's usually like that. everything flips! but some may want their component to open from the opposite side (it being rtl or ltr). So I think the component should be made a bit smarter to react to language direction. yet allow interception with openOpposite prop,

mbrookes commented 8 years ago

@alitaheri - sorry for the confusion, I was referring to your Stateless Functional Components comment, but by the time I submitted my comment, the conversation had moved on, so I deleted it.

Understand re RTL - @newoga had suggested openSecondary, which matches the language in the spec.

Regarding the overall component naming, Drawer to me says it opens and closes, where as it can be fixed (but yes, the spec also describes a fixed drawer!). SideNav seems to describe the general case better I think (although it has a slight air of fixedness about it that doesn't totally capture the default sliding behavior).

So, I dunno - either works better than LeftNav!

alitaheri commented 8 years ago

@mbrookes Yeah either one is good, I'm ok with both. and if openSecondary is the convention then sure :grin:

oliviertassinari commented 8 years ago

LeftNav is a bit of a misnomer.

I agree. But I'm confused:

Should we vote :ticket:?

newoga commented 8 years ago

I vote <SideNav />.

The material spec isn't exactly clear or perfect when distinguishing betweens patterns and components. But for this, they use the term Side nav in their layout documentation, and Navigation drawer in their patterns documentation (I take it that Navigation drawer is one possible implementation of the Side nav). Since the way a typical material-ui user would implement a navigation drawer is using a combination of the SideNav and maybe nested menu components, I vote for <SideNav />.

This is a similar issue between<Toolbar /> and <AppBar /> where AppBar should really be a possible implementation of a Toolbar

newoga commented 8 years ago

MDL confuses me a lot because it seems at times to invent new terms for things in the spec. Mmm...

mbrookes commented 8 years ago

There are quite a few ambiguities and even straight up contradictions in the spec, not just naming / terminology, (It's a shame it isn't open source, or at least had an 'issues' tracker somewhere), but the more I read these particular sections the more confused I become.

"Side nav bars may be pinned for permanent display, or they can float temporarily as overlays. Temporary nav drawers overlay the content canvas; whereas pinned nav panels" ref

So generically it's a "Side nav bar", if it's temporary it's a "Nav drawer", and if it's pinned it's a "nav panel"?

No, if it's permanent it's a "navigation drawer":

"Permanent navigation drawers are always visible and pinned to the left edge, at the same elevation as the content or background. " ref

Except:

"Persistent navigation drawers can toggle open or closed." ref

WTF?!

So when is a drawer not a drawer, and is this component an example of one? What if in the future we want to support the mini version, and maybe transitioning from mini to full size, or one of the other variants? If this is the definitive "drawer" component, will one component have to support that?

All this just to rename a component? I wish I hadn't asked! :laughing:

PS. @newoga Funny you should mention Toolbar vs AppBar - I noticed the same thing as I was going back and forth over the spec. I think there's an open issue around that topic.

newoga commented 8 years ago

@mbrookes Haha! That gave me a good laugh. :laughing: I feel your pain.

(It's a shame it isn't open source, or at least had an 'issues' tracker somewhere), but the more I read these particular sections the more confused I become.

I feel the same way.

As a general rule, I think we should do our best to "honor" the spec but we shouldn't kill ourselves in an attempt to conform to it perfectly. Based on my personal experience reading the spec, I'm thoroughly convinced that a 1-1 mapping is going to be impossible :laughing: (and I really think that it's okay if we don't).

That all being said, I do think material design (and its spec) is much more than the framework or the components. It's also about how you use them appropriately (something that material-ui can't enforce). The best example I can think of is the floating action button. Just because that material-ui framework makes making floating action buttons easy, it doesn't mean that you should put a dozen floating action buttons on the same page.

I think it is worthwhile to encourage developers that use material-ui to become knowledgable of the spec and using close/similar naming conventions will help them transfer knowledge between the spec and this project more easily. For example, I think both SideNav and NavigationDrawer are more appropriate than alternatives, because both can be found on the spec site. In our docs we can easily say here is how you do it in React, and then link to the spec site to say this is how you do it in your application.

alitaheri commented 8 years ago

I'm against sticking exactly to the specs. After all they are only guidelines in my opinion, it's good to follow conventions but we shouldn't limit ourselves to them. I like SideNav more, since it's easier to write and harder to misspell :grin: :+1:

oliviertassinari commented 8 years ago

I take it that Navigation drawer is one possible implementation of the Side nav

I agree. In the spec, we can see that it used to display picture information. Hence, I think that Navigation or Nav is out.

I vote for <DrawerLayout />, just to follow react-native :grin:.

Funny you should mention Toolbar vs AppBar - I noticed the same thing as I was going back and forth over the spec.

In the same spirit. I think that our existing AppBar should be the actual Toolbar(https://facebook.github.io/react-native/docs/toolbarandroid.html#content) and that the fixed positioning, waterfall effect, etc, should be handled by a new AppBar component.

leesei commented 8 years ago

To throw more stones into the water, it's Android that uses DrawerLayout to implement the Navigation Drawer pattern. And react-native just followed suit. http://developer.android.com/training/implementing-navigation/nav-drawer.html Polymer's Paper uses DrawerPanel. https://elements.polymer-project.org/elements/paper-drawer-panel

I'm beginning to believe you can name a component whatever you like as long as it is stated that it implements a certain Material design pattern.

mbrookes commented 8 years ago

So how about just Drawer? No inference of a specific use case, no mention of sidedness...

alitaheri commented 8 years ago

Drawer seems a lot nicer than all the other dialects of Drawer :D :D But I for one would be confused. English is not my native language and simpler words such as SideBar SideNav or LeftNav make more sense to me. I think it would also apply to every other nationality as well. I find SideNav very precise in describing the behavior of this component. But if we decide that the name should contain Drawer, then Drawer itself is perfect :+1: :+1:

mbrookes commented 8 years ago

Of those choice SideBar seems most descriptive and least prescriptive.

"There are two hard things in computer science: cache invalidation, and naming things" Phil Karlton

Did someone suggest a taking vote? :laughing:

alitaheri commented 8 years ago

@mbrookes That quote xD

voting seems good. any good pooling service you know of we go and easily vote? or just plain ol' :+1: <MY_VOTE>?

mbrookes commented 8 years ago

Hope I captured the main options:

https://www.surveymonkey.co.uk/r/LVMD9YR

newoga commented 8 years ago

Haha, I suppose the worst thing that could happen (that won't happen) is we get into a fiery angry debate and we all create our own forks with different names for this component :sweat_smile:. Thanks for making the survey @mbrookes, I'll be sure to vote soon.

But first out of curiosity: @oliviertassinari, if you were to make this component for react-native, would it re-use/wrap react native's DrawerLayout component, or would it be made separately from scratch? I could argue that one reason not to call it DrawerLayout is so we don't name clash with react-native... :smile: though I would really defer to your opinion on this matter.

mbrookes commented 8 years ago

:grinning: I hope not!

If there's a clear preference, we go for it, if not, we can have another go around.

You can revisit and change your order of preference if you change your mind, or are undecided / ambivalent about any of the options.

oliviertassinari commented 8 years ago

if you were to make this component for react-native, would it re-use/wrap react native's DrawerLayout component, or would it be made separately from scratch?

I think that we can simply use the DrawerLayout provided by react-native. Let's vote :tada:.

mbrookes commented 8 years ago

@shaurya947 @hai-cea @Zadielerick - your input would be welcomed!

https://www.surveymonkey.co.uk/r/LVMD9YR

mbrookes commented 8 years ago

Last call for votes. I'll close the survey and publish the results tomorrow.

mbrookes commented 8 years ago

So, the results are in, and the winner is...

(Drumroll please)...

:balloon: :tada: SideNav! :tada: :balloon:

So after all that, we're back to the original proposal! :astonished:

SideNav:

There were 6 responses. The choices were randomised (with only LeftNav being shown last). While I could see the results, I voted first and didn't change my vote afterwards.

(And FWIW, after taking all the feedback into account, I didn't vote SideNav first in the end - if I had, it would have come out even higher!)

So, there you have it. Let the fights commence! :rage1:

( :trollface: )

alitaheri commented 8 years ago

So, there you have it. Let the fights commence!

:laughing: :laughing: :laughing:

I like SideNav :grin: :+1:

nathanmarks commented 8 years ago

I don't know what the current verdict is, but I definitely prefer <Drawer />, <DrawerLayout /> or <NavDrawer />.

One of the primary reasons is that the drawer term is used a lot in the material spec and implies that it opens and closes.

I understand that it can be fixed, but remember that is desktop only and material design is device agnostic at it's core, not a desktop layout system.

Also, I strongly agree with @oliviertassinari's assertions RE the <AppBar />. Is there an issue for that?

mbrookes commented 8 years ago

I strongly agree with @oliviertassinari's assertions RE the .

@newoga has WIP on that.

alitaheri commented 8 years ago

Guys can we move this to the 0.15.0 release?

@callemall/material-ui I wanna compose another alpha.

nathanmarks commented 8 years ago

@alitaheri makes sense if the directory and file re-org is being done for 0.15.0.

alitaheri commented 8 years ago

Ok then :grin:

nathanmarks commented 8 years ago

@oliviertassinari Raised a very good point against using Nav as part of the name. But @alitaheri was concerned about the clarity. Do we have a consensus here?

@mbrookes I vote +1 for Drawer, so it's now tied with SideNav :stuck_out_tongue:

alitaheri commented 8 years ago

NavSideDrawer? :laughing: :laughing: Just thinking out of box here :sunglasses:

nathanmarks commented 8 years ago

@alitaheri haha, too long!

Oli's main concern was that the Drawer is not only for a navigation, that navigation is just one possible implementation for the drawer. This is pretty valid imo.

alitaheri commented 8 years ago

Hmmmmm Yeah! I agree with you :+1: :+1:

You've whipped my vote :laughing: So SideDrawer?

nathanmarks commented 8 years ago

@alitaheri I think that is better than SideNav. The material spec is all over the place for the naming of the Drawer or SideDrawer (I kinda like AppDrawer too, meshes well with AppBar).

Cause this just doesn't make sense :smile: image

nathanmarks commented 8 years ago

@alitaheri Also, AppDrawer can be used in a similar way to popover, other abstractions could be made using it such as BottomSheet, SideNav, etc... Just a random thought :smile:

image

For eg, with SideDrawer we cannot make an expandable footer drawer or "bottom sheet"

alitaheri commented 8 years ago

I really don't know :laughing: Let's call it Drawer and be done with it :laughing:

nathanmarks commented 8 years ago

@alitaheri Not AppDrawer? :stuck_out_tongue_winking_eye: :smile: :+1: ok, everyone fine with Drawer? @callemall/material-ui

alitaheri commented 8 years ago

I'm ok with both, but I like less typing :laughing:

newoga commented 8 years ago

Guys, I'm happy with <Drawer /> as well. I'm less opposed to it now as I was before. I also don't mind AppDrawer but I might prefer just regular Drawer. I think an application can possibly have multiple Drawers or SideNavs so might be better to not have App in the name as it might imply there can only be one.

Either way, I also good getting this in for this upcoming release :+1:

nathanmarks commented 8 years ago

@newoga Let's do it then! Who's doing the rename? Remember to tend to docs, examples, etc.

bravo-kernel commented 8 years ago

Thanks for renaming this to Drawer, makes a lot of sense to me!

sdmunoz commented 8 years ago

Good choice