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.

Sign up for GitHub

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

Jump to bottom

[editor] add misc toggle commands to the view menu #6843

Merged
merged 3 commits into from
Jan 9, 2020
Merged

[editor] add misc toggle commands to the view menu #6843

merged 3 commits into from
Jan 9, 2020

Conversation

vince-fugnitto
Copy link
Member

@vince-fugnitto vince-fugnitto commented Jan 7, 2020 •
edited
Loading

What it does

View: Toggle Word Wrap:

Fixes #2433

Adds the command, menu item, and keybinding which is used to toggle between the available word wrap values: (off, on, wordWrapColumn, bounded).

View Toggle Minimap:

Adds the missing view menu item to toggle the display of the minimap.
Adds the isToggled property to display the checkmark when the setting is on/off.

View: Toggle Render Whitespace:

Adds the command, and menu item to control the rendering of whitespace characters in the editor.
Adds the isToggled property to display the checkmark when the setting is on/off.

How to test

  • verify the command toggle word wrap (should properly cycle between possible values)
  • verify the command toggle minimap (should properly turn the minimap on/off)
  • verify the command toggle render whitespace (should properly turn the rendering of whitespace characters on/off)

Review checklist

Reminder for reviewers

@vince-fugnitto
[editor] add 'toggle minimap' command and menu
4dc9ccb 
- adds the `toggle minimap` menu with the toggle functionality.
- introduces a new menu group called `CommonMenus.VIEW_TOGGLE`.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto
[editor] add 'toggle render whitespace' command
67650f9 
- adds the `toggle render whitespace` command and menu item.
- adds the functionality to display the command as a toggle.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto added enhancement issues that are enhancements to current functionality - nice to haves editor issues related to the editor commands issues related to application commands labels Jan 7, 2020
@vince-fugnitto vince-fugnitto self-assigned this Jan 7, 2020
lmcbout
lmcbout reviewed Jan 8, 2020
View reviewed changes
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

One thing, when the "View" pulldown menu is opened and the user open the command palette with "CTRL+ SHIFT+P", the pulldown-menu does not go away, so using the command to "Render White space" does nor refresh the toggle in the pulldown-menu until you move the mouse outside the pulldown-menu.
May be we should close the pulldown menu when activating the command palette. (Same as VSCode)

@vince-fugnitto
Copy link
Member Author

One thing, when the "View" pulldown menu is opened and the user open the command palette with "CTRL+ SHIFT+P", the pulldown-menu does not go away, so using the command to "Render White space" does nor refresh the toggle in the pulldown-menu until you move the mouse outside the pulldown-menu.
May be we should close the pulldown menu when activating the command palette. (Same as VSCode)

That is very much out-of-scope for the pull request and exists already in master.
I think opening a general issue (not in the pull request as it'd get lost) is the best course of action so we can fix the problem.

@lmcbout lmcbout mentioned this pull request Jan 8, 2020
Closed
lmcbout
lmcbout approved these changes Jan 8, 2020
View reviewed changes
Copy link
Contributor

@lmcbout lmcbout left a comment

Choose a reason for hiding this comment

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

Created a new issue to track the command palette overlay the pull-down menu.
Works fine with Chrome on UBUNTU

elaihau
elaihau approved these changes Jan 9, 2020
View reviewed changes
Copy link
Contributor

@elaihau elaihau left a comment

Choose a reason for hiding this comment

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

Tested in example browser.

packages/editor/src/browser/editor-navigation-contribution.ts Outdated Show resolved Hide resolved
@vince-fugnitto
[editor] add command 'toggle word wrap'
8519c5a 
Fixes #2433

- added command `toggle word wrap` which toggles the word wrap
value from all available values present in the preference.

Signed-off-by: Vincent Fugnitto <vincent.fugnitto@ericsson.com>
@vince-fugnitto vince-fugnitto merged commit 2ef80c0 into master Jan 9, 2020
@vince-fugnitto vince-fugnitto deleted the vf/toggle-commands branch January 9, 2020 12:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@akosyakov akosyakov akosyakov left review comments

@elaihau elaihau elaihau approved these changes

@lmcbout lmcbout lmcbout approved these changes

Assignees

@vince-fugnitto vince-fugnitto

Labels
commands issues related to application commands editor issues related to the editor enhancement issues that are enhancements to current functionality - nice to haves
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add 'Toggle word wrap' command
4 participants
@vince-fugnitto @lmcbout @akosyakov @elaihau