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

[LeftNav] rename to SideNav #2697

Closed
mbrookes opened this issue Dec 29, 2015 · 46 comments · Fixed by #3799
Closed

[LeftNav] rename to SideNav #2697

mbrookes opened this issue Dec 29, 2015 · 46 comments · Fixed by #3799
Labels
design: material This is about Material Design, please involve a visual or UX designer in the process discussion

Comments

@mbrookes
Copy link
Member

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
Copy link
Contributor

newoga commented Dec 29, 2015

I agree with this. 👍 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
Copy link
Member

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

@alitaheri
Copy link
Member

It's Stateless Functional Components all over again 😞

@newoga
Copy link
Contributor

newoga commented Dec 29, 2015

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 />? 😆

@alitaheri
Copy link
Member

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 😆 😆

@alitaheri
Copy link
Member

@mbrookes Well in case of RTL language the <TARGET_NAME> will have to open from the right side. It's usually like that. everything flips! but some may want their <TARGET_NAME> 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
Copy link
Member Author

@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
Copy link
Member

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

@oliviertassinari
Copy link
Member

LeftNav is a bit of a misnomer.

I agree. But I'm confused:

Should we vote 🎫?

@newoga
Copy link
Contributor

newoga commented Dec 29, 2015

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
Copy link
Contributor

newoga commented Dec 29, 2015

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

@mbrookes
Copy link
Member Author

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! 😆

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
Copy link
Contributor

newoga commented Dec 30, 2015

@mbrookes Haha! That gave me a good laugh. 😆 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 😆 (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
Copy link
Member

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 😁 👍

@oliviertassinari
Copy link
Member

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 😁.

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.

@alitaheri alitaheri added design: material This is about Material Design, please involve a visual or UX designer in the process discussion labels Dec 30, 2015
@leesei
Copy link
Contributor

leesei commented Dec 31, 2015

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
Copy link
Member Author

mbrookes commented Jan 5, 2016

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

@alitaheri
Copy link
Member

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 👍 👍

@mbrookes
Copy link
Member Author

mbrookes commented Jan 5, 2016

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? 😆

@alitaheri
Copy link
Member

@mbrookes That quote xD

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

@mbrookes
Copy link
Member Author

mbrookes commented Jan 5, 2016

Hope I captured the main options:

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

@newoga
Copy link
Contributor

newoga commented Jan 5, 2016

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 😅. 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... 😄 though I would really defer to your opinion on this matter.

@mbrookes
Copy link
Member Author

mbrookes commented Jan 5, 2016

😀 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
Copy link
Member

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 🎉.

@mbrookes
Copy link
Member Author

mbrookes commented Jan 6, 2016

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

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

@mbrookes
Copy link
Member Author

mbrookes commented Jan 7, 2016

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

@mbrookes
Copy link
Member Author

mbrookes commented Jan 7, 2016

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

(Drumroll please)...

🎈 🎉 SideNav! 🎉 🎈

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

SideNav:

  • was first over all with a score of 6.5 out of 8. (Drawer was second with 5.8 out of 8.)
  • had most first preferences votes (3 votes out of 8). (The three others that received a first preference vote [Drawer, DrawerLayout, SidePanel] only got 1 first place vote each).
  • received a lowest vote of 3 out of 8, so no-one totally disliked it. (Most scored at least one 1 or 2 out of 8 , with only SideDrawer doing better with a lowest score of 5 out of 8).

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
Copy link
Member

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

😆 😆 😆

I like SideNav 😁 👍

@nathanmarks
Copy link
Member

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
Copy link
Member Author

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

@newoga has WIP on that.

@alitaheri
Copy link
Member

Guys can we move this to the 0.15.0 release?

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

@nathanmarks
Copy link
Member

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

@alitaheri
Copy link
Member

Ok then 😁

@nathanmarks
Copy link
Member

@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 😛

@alitaheri
Copy link
Member

NavSideDrawer? 😆 😆 Just thinking out of box here 😎

@nathanmarks
Copy link
Member

@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
Copy link
Member

Hmmmmm Yeah! I agree with you 👍 👍

You've whipped my vote 😆 So SideDrawer?

@nathanmarks
Copy link
Member

@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 😄
image

@nathanmarks
Copy link
Member

@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 😄

image

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

@alitaheri
Copy link
Member

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

@nathanmarks
Copy link
Member

@alitaheri Not AppDrawer? 😜 😄 👍 ok, everyone fine with Drawer? @callemall/material-ui

@alitaheri
Copy link
Member

I'm ok with both, but I like less typing 😆

@newoga
Copy link
Contributor

newoga commented Mar 16, 2016

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 👍

@nathanmarks
Copy link
Member

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

nathanmarks pushed a commit to nathanmarks/material-ui that referenced this issue Mar 25, 2016
Rename `LeftNav` component to `Drawer` to better reflect the MD Spec
and broader use cases than navigation.

Also rename `openRight` propery to `openSecondary` to make RTL use less confusing.

Update docs site examples to use the new component, and update wording.

Rename docs site `AppLeftNav` to `AppNavDrawer`, and rename all internal functions.

Closes mui#2697.
nathanmarks pushed a commit to nathanmarks/material-ui that referenced this issue Mar 25, 2016
Rename `LeftNav` component to `Drawer` to better reflect the MD Spec
and broader use cases than navigation.

Also rename `openRight` propery to `openSecondary` to make RTL use less confusing.

Update docs site examples to use the new component, and update wording.

Rename docs site `AppLeftNav` to `AppNavDrawer`, and rename all internal functions.

Closes mui#2697.
@bravo-kernel
Copy link

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

@sdmunoz
Copy link

sdmunoz commented Apr 2, 2016

Good choice

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
design: material This is about Material Design, please involve a visual or UX designer in the process discussion
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants