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

fix #8226: Add a settings menu on left sidebar bottom #8372

Merged
merged 1 commit into from
Aug 26, 2020
Merged

fix #8226: Add a settings menu on left sidebar bottom #8372

merged 1 commit into from
Aug 26, 2020

Conversation

datou0412
Copy link
Contributor

@datou0412 datou0412 commented Aug 13, 2020

Signed-off-by: 二凢 dingyu.wdy@alibaba-inc.com

What it does

Fixes: #8226

Add a settings menu on left sidebar bottom.

image

How to test

  1. Open a workspace.
  2. Click the cog icon on the bottom of left sidebar, open the new Settings menu.
  3. Click the menu item, 'Open Preferences' for instance, and it works.

Review checklist

Reminder for reviewers

Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The idea is not to remove the file > settings menu item, but instead add a new way for end-users to access the menu items in the form of the sidepanel icon:

Example:

settings-a

settings-b

@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves preferences issues related to preferences labels Aug 13, 2020
@datou0412
Copy link
Contributor Author

The idea is not to remove the file > settings menu item, but instead add a new way for end-users to access the menu items in the form of the sidepanel icon:

Example:

settings-a

settings-b

keep both?

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Aug 13, 2020

keep both?

Yes, vscode does the same, the idea is not to force end-users to use one solution or the other but provide alternative ways for them to be able to access the menu. Removing the menus from File is a breaking behavior.

@datou0412
Copy link
Contributor Author

The Mac version VSCode dose not have a Settings submenu of Files

image

@vince-fugnitto
Copy link
Member

vince-fugnitto commented Aug 13, 2020

The Mac version VSCode dose not have a Settings submenu of Files

Then the solution should be operating-system specific like we have in other areas.
It is present on Linux (which is the screencasts I shared), and for Linux users they'd expect it.

Update: I believe it is present in vscode, just under a different menu, see: #6497

@datou0412 datou0412 changed the title fix #8226: mv settings menu to left sidebar bottom fix #8226: Add a settings menu on left sidebar bottom Aug 13, 2020
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@datou0412 thank you for updating the pull-request and restoring the top-level menu for settings 👍
The changes work very well, I verified that:

  • each command works successfully
  • the sidepanel entry for settings cannot be moved, and is always visible

I think we can think about adding more entries like command palette in future iterations as not to hold back the changes.

I'll also give others a chance to review as well 👍

packages/core/src/browser/style/sidepanel.css Outdated Show resolved Hide resolved
@akosyakov akosyakov self-requested a review August 13, 2020 14:58
@vince-fugnitto vince-fugnitto self-requested a review August 14, 2020 12:21
@datou0412
Copy link
Contributor Author

Need a re-review @akosyakov

Copy link
Member

@akosyakov akosyakov left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wow that's very clean work, could you address comments if you are agree and we merge

Signed-off-by: 二凢 <dingyu.wdy@alibaba-inc.com>
@datou0412
Copy link
Contributor Author

wow that's very clean work, could you address comments if you are agree and we merge

Of course, u r the boss :D

@datou0412 datou0412 requested a review from akosyakov August 26, 2020 11:55
@akosyakov akosyakov merged commit d7f9163 into eclipse-theia:master Aug 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement issues that are enhancements to current functionality - nice to haves preferences issues related to preferences
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a setting menu on left-side-bar-bottom
3 participants