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

Should terminal font honor user's Monospace choices in the browser? #1317

Closed
fperez opened this issue Nov 29, 2016 · 6 comments · Fixed by #1377
Closed

Should terminal font honor user's Monospace choices in the browser? #1317

fperez opened this issue Nov 29, 2016 · 6 comments · Fixed by #1377
Assignees
Labels
pkg:terminal status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:Design and UX
Milestone

Comments

@fperez
Copy link
Contributor

fperez commented Nov 29, 2016

I know full-blown font/theme terminal customization is a can of worms, but I wonder if allowing at least the terminal component to honor the user's choice of monospace font in the browser makes sense. I just tried and it didn't have any impact, at least on Chrome on OSX.

My rationale is that we can imagine the user has chosen a mono font they otherwise like and that works well for them in general, so giving them that baseline may do the 80/20 job on this issue...

Thoughts?

@blink1073
Copy link
Member

IIUC this is a matter of setting font-family: monospace instead of the more specific specifier here:

font-family: "DejaVu Sans Mono", "Liberation Mono", monospace;
.

cc @cameronoelsen, thoughts on this?

@blink1073 blink1073 added this to the Beta milestone Dec 2, 2016
@cameronoelsen
Copy link
Contributor

@blink1073 @fperez How many users have a monospace font preference for terminals? Like the only way I would see this being incorporated would be in the top menus in the terminal settings. I just don't know how many users would use it

@fperez
Copy link
Contributor Author

fperez commented Dec 2, 2016

No, the point is that users are likely to have their browser-wide monospace font selected, and we should honor that. Right now, by having explicit choices in advance, it seems that if those aren't found, the default user's choice gets overridden.

At least that's what I'm observing on OSX with Chrome 54: I have menlo as my monospace font, but instead at the terminal I'm getting something else that looks hideous. I don't have DejaVu Sans Mono available, so it seems that Chrome, instead of falling back to my monospace choice, is instead picking some internal fallback (Courier New, I think).

I know this is a tricky one, perhaps you prefer to provide actual JLab-specific customization. I'm thinking of having a reasonable fallback for the folks at large who won't look into customizing JLab specifically but simply set their browser preferences once and forget about it everafter.

@fperez
Copy link
Contributor Author

fperez commented Dec 5, 2016

Yup, just to confirm that the small change suggested by @blink1073 does the trick. See here, current master at the top, with the change at the bottom:

image

Now, looking at the code makes me wonder why that "DejaVu Sans Mono", "Liberation Mono" declaration is there at all, esp. in light of the L26 below, whose intent seems to be to allow the monospace declaration to be the default. Is it possible that L18 is a stale remnant?

If so, I'm happy to make the (trivial, obviously :) cleanup PR...

@blink1073
Copy link
Member

Found where it came from, though I don't know what my original logic was (other than Googling about how to set a monospace font), I'll open the PR to fix it.

jupyter/jupyter-js-terminal@c8f3210#diff-b3c787053edaeedadf1a2430f41506f2

@fperez
Copy link
Contributor Author

fperez commented Dec 5, 2016

Great, thx!

@lock lock bot added the status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. label Aug 10, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Aug 10, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pkg:terminal status:resolved-locked Closed issues are locked after 30 days inactivity. Please open a new issue for related discussion. tag:Design and UX
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants