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

Make Settings and Keybindings editor consistent in showing editor actions #61590

Closed
isidorn opened this Issue Oct 23, 2018 · 23 comments

Comments

@isidorn
Copy link
Contributor

isidorn commented Oct 23, 2018

@sandy081 and me find it weird that we have a custom place in settings where we keep secondary actions and we do not use editor secondary actions. Why is that, what is the reasoning behind this design choice?
This way we are inconsistent with out other settings experiences, for example Keybinginds Editor.

fyi @misolori

screen shot 2018-10-23 at 12 08 11

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Oct 23, 2018

I think we were concerned that the editor gutter actions were too far away. The settings editor is different because its content is centered so there's no content near the common actions, like your screenshot shows. But I'm thinking I should just use that spot anyway.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 23, 2018

I would go with consistency here. Especially since these actions are not for novice users but for expeirenced users who are aware of editor primary and secondary actions.

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 23, 2018

The other reason we decided to not use the editor actions was that majority of the actions modify your window/view (markdown preview, diff, changes, etc.) and Settings had filters that are more contextual and are similar to the extension search + context menu.

Also, we had issues with the discoverability of those actions that usually don't contain additional actions.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 23, 2018

Open settings.json also modifies my view and is very similar to Git show initial file / diff which is in the editor title area.

For discoverability do we maybe have some numbers to back that claim up that these makes the settings more discoverable?

Please be aware of this one also and that we shuold be consistent #61587
Our keybindins and settings experience need to follow the same patterns and imho we should invest there. If the user figures out one he should not learn differnt patterns all over again

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 23, 2018

I agree that open settings.json is similar to those other views.

We can definitely grab numbers to see how many people are accessing the json via the current dropdown, but I think a more appropriate way to measure that is to do a short usability study, since this will also apply to the keybindings page. Maybe @stevencl has thoughts on how long it would take to get feedback from users.

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 23, 2018

I think we're in agreement that the experiences should be the same/consistent. Here are the two different methods, @roblourens @isidorn @sandy081 let me know if this aligns with your thinking:

Use editor actions

This aligns with the current keybindings page and moves the "Open settings.json" into an action/icon and adds the filters to the editor context menu.

image

Use contextual actions

This is how the current settings page is except we'd still keep the "Open settings.json" in the editor actions.

image

@sandy081 sandy081 changed the title Settings: why do we have custom place for actions and not use editor secondary actions Make Settings and Keybindings editor consistent in showing editor actions Oct 23, 2018

@sandy081 sandy081 self-assigned this Oct 23, 2018

@sandy081

This comment has been minimized.

Copy link
Member

sandy081 commented Oct 23, 2018

@misolori I liked the first one that uses editor actions. This will make it consistent with other editors too. Show two ... looks to me redundant and not good.

Regarding opening json file action, if we show it as primary action then we should need good icon to identify the action. Otherwise we can move it as secondary action similar to how it is now in Settings editor but as editor actions.

@misolori misolori self-assigned this Oct 23, 2018

@misolori misolori added the ux label Oct 23, 2018

@stevencl

This comment has been minimized.

Copy link
Member

stevencl commented Oct 23, 2018

Is a study required? Seems like there is general consensus that consistency is preferred?

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 23, 2018

@stevencl doesn't look like we'll need a study, we'll just monitor the feedback with this change.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 24, 2018

@misolori thanks for skethiching that up. Curly bracket icon might just work.
It would be great if we could get this into insiders so we get feedback and potnetialy change before we release in 10 days.

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Oct 24, 2018

The keybindings editor uses the file/arrow
image

I like the braces too. Chrome devtools uses that icon for "Pretty print", I don't really know why.

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 24, 2018

We also use that icon in the Markdown preview as the "Go to source" and think we can use either. However, in #61587 there was discussion that the file/arrow icon was hard to notice and often missed.

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Oct 25, 2018

File/arrow icon was unnoticable by everybody in zurich that tried to find this functionality - it associates me to the "git get me out of diff view", since we use it there already.

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Oct 26, 2018

I can do this if you send me the braces icon @misolori ?

@sandy081 sandy081 added this to the November 2018 milestone Oct 26, 2018

@sandy081

This comment has been minimized.

Copy link
Member

sandy081 commented Oct 26, 2018

I am planning this to next milestone so that we can run with these actions in insiders to see if there are no complains.

@misolori

This comment has been minimized.

Copy link
Member

misolori commented Oct 26, 2018

@roblourens sent you the icons

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Oct 26, 2018

Sounds good @sandy081

@isidorn

This comment has been minimized.

Copy link
Contributor

isidorn commented Dec 4, 2018

The hover titles can still be aligned @roblourens and @sandy081
Though the overall alignment is good adding verified

@sandy081

This comment has been minimized.

Copy link
Member

sandy081 commented Dec 4, 2018

I updated Keyboard shortcuts actions title to Open Keyboard Shortcuts and Open Keyboard Shortcuts (JSON). Did not add (UI) part as Open Keyboard Shortcuts is the default command to open the editor.

image

@roblourens Same can be done for Settings editor actions?

sandy081 added a commit that referenced this issue Dec 4, 2018

@usernamehw

This comment has been minimized.

Copy link
Contributor

usernamehw commented Dec 4, 2018

@sandy081, can those items not be named that way (in parentheses). vscode search not matching them: #27636 and there is no progress on that issue. It would be ideal for the time being not using brackets at all.

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Dec 4, 2018

@sandy081 looks like they match now so there's nothing to do for settings right?

The filtering issue is annoying...

@sandy081

This comment has been minimized.

Copy link
Member

sandy081 commented Dec 5, 2018

Yeah they match now, but I think Settings has another action called Open Settings (UI).

sandy081 added a commit that referenced this issue Dec 5, 2018

@roblourens

This comment has been minimized.

Copy link
Member

roblourens commented Dec 5, 2018

Now there are two different actions with the same name, I'm reverting that commit. Settings has 3 - open the configured editor, open the settings ui, or open the settings json.

roblourens added a commit that referenced this issue Dec 5, 2018

Revert "#61590 Change the title"
This reverts commit 22fef36.

@vscodebot vscodebot bot locked and limited conversation to collaborators Dec 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.