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

Rename box does not respect editor font #5226

Closed
isidorn opened this issue Apr 13, 2016 · 15 comments
Closed

Rename box does not respect editor font #5226

isidorn opened this issue Apr 13, 2016 · 15 comments
Assignees
Labels
ux User experience issues

Comments

@isidorn
Copy link
Contributor

isidorn commented Apr 13, 2016

Try to rename any variable in TS, notice that the rename box is not respecting the font in the editor.
This is like this since forever but not sure if there is an issue already for this

screen shot 2016-04-13 at 16 55 25

@isidorn isidorn added the bug Issue identified by VS Code Team member as probable bug label Apr 13, 2016
@jrieken jrieken added the ux User experience issues label Apr 13, 2016
@jrieken
Copy link
Member

jrieken commented Apr 13, 2016

Unsure if it should be like that at all? @bgashler1 Ideas? Answers?

@jrieken
Copy link
Member

jrieken commented Apr 15, 2016

Comparing this to search/replace I don't think we should do it. Please reopen when and explain why this shouldn't be like search/replace or quick outline/

@jrieken jrieken closed this as completed Apr 15, 2016
@isidorn
Copy link
Contributor Author

isidorn commented Apr 15, 2016

Arguments that this should respect the code font:

  • lives more connected to the source code than search replace
  • always has the variable name as the content
  • visually not appeleaing to have two different fonts so close to each other

Thinking about it, I would vote that search / replace and quick outline also respect editor fonts. What is the reasoning for those to have different fonts?
QuickOpen makes sense to have different font when displaying titles of files, but not when displaying variable names imho.

Feel free to leave closed if you do not think the arguments are strong enough.

@bpasero @stevencl opinions welcome

@stevencl
Copy link
Member

I have to admit I had never really noticed that before. I don't feel very strongly about it. Consistency is good though, so if it was a simple, inexpensive change to make I would be for it. If it was to change, I'd expect it to change in search too.

@bgashler1
Copy link
Contributor

Sorry I let this one slip by me in not replying quickly. I also agree with Steven that consistency is key.

I tend to lean towards not having the monospaced code font in the input box. The reason why is that the input box is a UI overlay, and UI has its own non-monospaced font. We're not editing inline—we're merely entering something as input to the overlay which then makes the change on our behalf.

@isidorn
Copy link
Contributor Author

isidorn commented Apr 20, 2016

Then it makes sense to keep this closed, thanks for feedback.

@jrieken
Copy link
Member

jrieken commented Apr 20, 2016

so who did this change then?

screen shot 2016-04-20 at 10 56 04

@joaomoreno
Copy link
Member

joaomoreno commented Apr 20, 2016

I just saw the monospace font and I love it. It always felt weird to me that it was non-monospace, regardless of the other input boxes. The same goes for the find & replace.

@isidorn
Copy link
Contributor Author

isidorn commented Apr 20, 2016

I did not change it, but I do like it. Thanks misterious contributor

@jrieken
Copy link
Member

jrieken commented Apr 20, 2016

Sorry, reverted. IMO consistency goes over personal preference

@joaomoreno
Copy link
Member

Consistency goes both ways.

You seem to be on the side of widget consistency: most text input widgets show non-monospace font, so this one should to. Examples are in the search and git viewlets' input boxes, quick open, find and replace. Counter examples are the debug evaluate input box and watch expressions, both of which use monospace font, imo correctly.

I don't agree with that type of consistency.

I feel content consistency is more important. Meaning: wherever code is always shown, monospace should be used; non-monospace otherwise. This already is very consistent nowadays: editor, output, suggest widget, parameter hints, hover widget; they all show monospace font. It is even consistent with quick open and the search and git viewlets' input boxes, since you do not always write code in there; you might write plain text, so that is non monospace.

This leads me to point out where this consistency is not followed: find all references, peek definition, search viewlet contents and, finally, the rename widget. All of these are examples in which I don't feel we're doing the right thing: they should all show monospace font.

@stevencl
Copy link
Member

That's a very compelling argument @joaomoreno, I agree.

@jrieken
Copy link
Member

jrieken commented Apr 20, 2016

I think we agree on consistency, I just disagree about making that change in small increments and starting with a random widget. If we can establish and formulate a rule then we should go by it.

@jrieken jrieken assigned stevencl and unassigned jrieken May 26, 2016
@jrieken
Copy link
Member

jrieken commented May 26, 2016

Assigning this to @stevencl to coordinate this effort across all the areas @joaomoreno mentioned above

@felixfbecker
Copy link
Contributor

felixfbecker commented May 31, 2016

I would also say that writing RegExps in the search box would be much easier with a monospaced font.

@isidorn isidorn removed the bug Issue identified by VS Code Team member as probable bug label Jul 1, 2016
@vscodebot vscodebot bot locked and limited conversation to collaborators Nov 18, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ux User experience issues
Projects
None yet
Development

No branches or pull requests

6 participants