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

Left Area -> Left Sidebar in menu #3818

Merged
merged 5 commits into from May 18, 2018
Merged

Left Area -> Left Sidebar in menu #3818

merged 5 commits into from May 18, 2018

Conversation

@jasongrout
Copy link
Contributor

@jasongrout jasongrout commented Feb 6, 2018

This builds on #3801 and adds one commit - merge #3801 first.

This changes the menu text to align with our terminology of "sidebar", i.e., the change is "Show Left Area" -> "Show Left Sidebar" in the menu and the analogous change in the command palette text. No APIs were harmed in the making of this PR.

In #3801 (comment), there was some discussion about what "sidebar" technically meant. Is it just the vertical tabs? Is it just the left panel content? Is it both?

The only real change in this PR beyond #3801 is 056359e, which just reverts one of the last commits of #3801.

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Feb 6, 2018

If we are looking at other uses of "sidebar", such as OS X apps, the sidebar is the actual content region on the left (and there is no analog to our vertical tabs that always are visible). That's why in OS X Preview, it is literally "Hide Sidebar" in the menu.

Same with OpenOffice and other apps: https://en.wikipedia.org/wiki/Sidebar_(computing) - the sidebar hosts the content:

Unlike toolbars and status bars, sidebars have larger surface areas because of horizontally longer layout of desktop apps. Sidebars may use accordions to organize widgets and accommodate a larger layout than the visible surface area.

Typically a sidebar has an accordion to organize sections, but we use vertical tabs. I think widespread accepted usage is that "sidebar" refers to the content area, though. Since we use tabs instead of accordions, it's a bit ambiguous to me whether "sidebar" in our case includes the tabs, or is just the content area (I would probably say it includes the tabs, i.e., is the entire left tab panel, and maybe terminology should be "expand/collapse sidebar").

Apparently there was still some misunderstanding when we defined terms before. @ellisonbg, you were in that discussion before too--what were you thinking "sidebar" meant?

@jasongrout jasongrout mentioned this pull request Feb 6, 2018
1 task
@jasongrout jasongrout added this to the Beta 1 milestone Feb 6, 2018
@jzf2101
Copy link
Contributor

@jzf2101 jzf2101 commented Feb 6, 2018

@jasongrout merged your old pr can you merge upstream?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Feb 6, 2018

It will work automatically...git will recognize and merge this properly

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Feb 6, 2018

We may not want to merge this right now since it will probably make one or two screenshots out of date. I think the language in the menu is 'good enough' that it isn't totally confusing, so I'll push this to beta 2.

@jasongrout jasongrout removed this from the Beta 1 milestone Feb 6, 2018
@jasongrout jasongrout added this to the Beta 2 milestone Feb 6, 2018
@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented Mar 15, 2018

Bumping to beta 3 to give a bit more time before we'd need to redo screenshots, if we decided to do this.

@jasongrout jasongrout removed this from the Beta 2 milestone Mar 15, 2018
@jasongrout jasongrout added this to the Beta 3 milestone Mar 15, 2018
@ian-r-rose
Copy link
Member

@ian-r-rose ian-r-rose commented Mar 21, 2018

I have no objection to using "Left Sidebar". @jasongrout, am I correct in thinking that most of these changes have already been merged? If so, can we open a new PR with just the name changes?

@jasongrout jasongrout removed this from the Beta 3 milestone Apr 18, 2018
@jasongrout jasongrout added this to the Beta 2 Patch milestone Apr 18, 2018
@jasongrout jasongrout removed this from the Minor release milestone Apr 28, 2018
@jasongrout jasongrout added this to the Beta 3 milestone Apr 28, 2018
@jzf2101 jzf2101 self-requested a review May 16, 2018
@jzf2101
Copy link
Contributor

@jzf2101 jzf2101 commented May 16, 2018

It appears a lot of these changes have already been adopted. What's the best way to test packages/application-extension/src/index.tsx?

@jasongrout
Copy link
Contributor Author

@jasongrout jasongrout commented May 18, 2018

Looks good to me!

@jzf2101 jzf2101 merged commit 90e5d7e into jupyterlab:master May 18, 2018
2 checks passed
@lock lock bot locked as resolved and limited conversation to collaborators Aug 9, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants