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

Change textBaseline from "ideographic" to "bottom" #3356

Closed
wants to merge 2 commits into from
Closed

Change textBaseline from "ideographic" to "bottom" #3356

wants to merge 2 commits into from

Conversation

dstein64
Copy link
Contributor

This changes the textBaseline from ideographic to bottom to resolve Issue #3353. I made changes in all places where middle was changed to ideographic as part of PR #3279.

In my testing, the update worked as expected. However, I don't know whether there are assumptions elsewhere that this would break. Any feedback would be appreciated if there is further testing I should conduct and/or additional changes that should be made.

I tried using the code in https://github.com/xtermjs/xterm.js/wiki/Powerline-fonts to test whether powerline fonts continue to work as expected, but the fonts were not supported by my browser, either with or without the change in this PR. I tried on Chrome as well with no success. I also tried on Windows instead of Ubuntu, and encountered the same issue.

fonts_not_supported

@dstein64
Copy link
Contributor Author

dstein64 commented May 30, 2021

I've installed Powerline-supported fonts.

While this update helps with Firefox, it appears to break the proper alignment in Chrome that was added in PR #3279.

Powerline on Firefox before this PR:

powerline_before_firefox

Powerline on Firefox after this PR:

powerline_after_firefox

Powerline on Chrome before this PR:

powerline_before_chrome

Powerline on Chrome after this PR:

powerline_after_chrome

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant