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

Add a warning to the proportional font selector #13389

Closed
givinghawk opened this issue Jun 27, 2022 · 12 comments · Fixed by #15195
Closed

Add a warning to the proportional font selector #13389

givinghawk opened this issue Jun 27, 2022 · 12 comments · Fixed by #15195
Labels
Area-SettingsUI Anything specific to the SUI good first issue This is a fix that might be easier for someone to do as a first contribution In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Milestone

Comments

@givinghawk
Copy link

Windows Terminal version

1.13.11431.0

Windows build number

10.0.19044.1766

Other Software

No response

Steps to reproduce

Open the program

Expected Behavior

Cursor on text at point of editing

Actual Behavior

Cursor is way off of where I am editing

image

@givinghawk givinghawk added the Issue-Bug It either shouldn't be doing this or needs an investigation. label Jun 27, 2022
@ghost ghost added Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting Needs-Tag-Fix Doesn't match tag requirements labels Jun 27, 2022
@DHowett
Copy link
Member

DHowett commented Jun 27, 2022

You appear to be using a proportional font in your terminal. This is not a supported configuration, and is why non-monospace fonts are hidden by default. :)

@DHowett DHowett added the Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons. label Jun 27, 2022
@DHowett DHowett closed this as not planned Won't fix, can't repro, duplicate, stale Jun 27, 2022
@givinghawk
Copy link
Author

ah! i did not realise. thanks!

@DHowett
Copy link
Member

DHowett commented Jun 27, 2022

No problem! We should actually add a warning! Thanks for bringing this to our attention.

If it seems weird that we offer a "break my terminal" checkbox that lets you select bad fonts, it is to work around a problem where a good font is mis-identified (by the OS) as an invalid monospace font.

@DHowett DHowett reopened this Jun 27, 2022
@DHowett DHowett changed the title Cursor is not showing where I am editing text Add a warning to the proportional font selector Jun 27, 2022
@DHowett DHowett added Product-Terminal The new Windows Terminal. Issue-Task It's a feature request, but it doesn't really need a major design. Area-SettingsUI Anything specific to the SUI and removed Resolution-By-Design It's supposed to be this way. Sometimes for compatibility reasons. Issue-Bug It either shouldn't be doing this or needs an investigation. Needs-Triage It's a new issue that the core contributor team needs to triage at the next triage meeting labels Jun 27, 2022
@ghost ghost removed the Needs-Tag-Fix Doesn't match tag requirements label Jun 27, 2022
@DHowett DHowett added the good first issue This is a fix that might be easier for someone to do as a first contribution label Jun 27, 2022
@DHowett DHowett added this to the Backlog milestone Jun 27, 2022
@zadjii-msft
Copy link
Member

@jamespack Hope you don't mind me saying, but this one feels kinda up the same alley of some of the other stuff you've been helping with 😉

@jamespack
Copy link
Contributor

Don't mind at all. Happy to PR it

@jamespack
Copy link
Contributor

image

this guy?

@zadjii-msft
Copy link
Member

That's the one. I'm looking through the settings app now to see if there's some precedent for like, a Warning InfoBar (within a Expander) if you choose a weird compatibility setting. Just for some UX precedent.

@zadjii-msft
Copy link
Member

image

Well, that's not exactly what I was looking for but that feels kinda close. Something big and yellow to say "Warning: non-monospaced will likely result in visual artifacts. Use at your own discretion" whenever the selected font isn't in MonospaceFontList

@jamespack
Copy link
Contributor

image

So not here then. I thought it would wrap 🤣

@jamespack
Copy link
Contributor

Something like this?

image

Top or bottom of the container? Id assume we want the text in the resources?

@zadjii-msft
Copy link
Member

That's literally exactly what I imagined. I'd put it underneath (but I'm no UX designer), and yea strings in the resources. You sure whipped that up quick 😛

@jamespack
Copy link
Contributor

#15195

Think I got everything where it needs to be. If that is not quite what you had in mind or I missed something let me know. :)

zadjii-msft pushed a commit that referenced this issue Apr 20, 2023
## Summary of the Pull Request
Add an infobar warning when a non-monospaced font is selected.
## References and Relevant Issues
#13389 
## Detailed Description of the Pull Request / Additional comments
I initially had the `IsOpen` property of the infobar bound to the
`ShowAllFonts` checkbox property. However, I felt we could do better by
adding a property for it since there was already a method defined to
inspect whether the selected font was in the `MonoSpaceFontList`.
## Validation Steps Performed
Warning shows up when a non-monospaced font is selected either globally
or on individual profiles. All existing tests continue to pass.
<img width="868" alt="image"
src="https://user-images.githubusercontent.com/2086722/232594214-cd42397b-ce9d-499c-aa73-3feaa45e850e.png">

## PR Checklist
- [x] Closes #13389 
- [x] Tests added/passed
- [ ] Documentation updated
- If checked, please file a pull request on [our docs
repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
- [ ] Schema updated (if necessary)
@microsoft-github-policy-service microsoft-github-policy-service bot added the Needs-Tag-Fix Doesn't match tag requirements label Apr 20, 2023
DHowett pushed a commit that referenced this issue Apr 25, 2023
## Summary of the Pull Request
Add an infobar warning when a non-monospaced font is selected.
## References and Relevant Issues
#13389
## Detailed Description of the Pull Request / Additional comments
I initially had the `IsOpen` property of the infobar bound to the
`ShowAllFonts` checkbox property. However, I felt we could do better by
adding a property for it since there was already a method defined to
inspect whether the selected font was in the `MonoSpaceFontList`.
## Validation Steps Performed
Warning shows up when a non-monospaced font is selected either globally
or on individual profiles. All existing tests continue to pass.
<img width="868" alt="image"
src="https://user-images.githubusercontent.com/2086722/232594214-cd42397b-ce9d-499c-aa73-3feaa45e850e.png">

## PR Checklist
- [x] Closes #13389
- [x] Tests added/passed
- [ ] Documentation updated
- If checked, please file a pull request on [our docs
repo](https://github.com/MicrosoftDocs/terminal) and link it here: #xxx
- [ ] Schema updated (if necessary)

(cherry picked from commit 2c16543)
Service-Card-Id: 89001996
Service-Version: 1.17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-SettingsUI Anything specific to the SUI good first issue This is a fix that might be easier for someone to do as a first contribution In-PR This issue has a related PR Issue-Task It's a feature request, but it doesn't really need a major design. Needs-Tag-Fix Doesn't match tag requirements Product-Terminal The new Windows Terminal.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants