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 settings for changing the workbench font size and font #144365

Closed

Conversation

ElijahPepe
Copy link

@ElijahPepe ElijahPepe commented Mar 4, 2022

This PR fixes #519.

This does need to be refined a bit; I did this rather hastily.

@ghost
Copy link

ghost commented Mar 4, 2022

CLA assistant check
All CLA requirements met.

@ElijahPepe
Copy link
Author

This PR adds the settings necessary for setting a font size and font family for the workbench and applies it in themeing.ts. I'm not sure if anything else is necessary like watching for changes to the settings and applies them retroactively.

@ElijahPepe ElijahPepe force-pushed the ElijahPepe/workbench-adjustments branch from 691cac3 to 904a1ee Compare March 5, 2022 03:18
@ElijahPepe
Copy link
Author

@sbatten, any word?

@ElijahPepe ElijahPepe force-pushed the ElijahPepe/workbench-adjustments branch from c926479 to 904a1ee Compare March 7, 2022 20:19
@sbatten sbatten removed their assignment Mar 8, 2022
@ElijahPepe ElijahPepe force-pushed the ElijahPepe/workbench-adjustments branch from 9607f58 to 904a1ee Compare March 8, 2022 20:43
@bpasero bpasero marked this pull request as draft March 9, 2022 06:21
@bpasero bpasero added this to the Backlog milestone Mar 9, 2022
@bpasero
Copy link
Member

bpasero commented Mar 9, 2022

Same discussion starting from #63602 (comment) applies here again.

@ElijahPepe
Copy link
Author

Same discussion starting from #63602 (comment) applies here again.

Ah, okay, I see the issue. I will attempt to find as many instances as I can of certain pixel values being hardcoded and alter them.

@ElijahPepe
Copy link
Author

I think I got all of the hardcoded values, but I'm not entirely sure.

@ElijahPepe ElijahPepe force-pushed the ElijahPepe/workbench-adjustments branch from 4c73c99 to 5c96bea Compare March 11, 2022 15:29
@ElijahPepe ElijahPepe marked this pull request as ready for review March 13, 2022 16:05
@ElijahPepe
Copy link
Author

Tests should be green now.

@laurentlb
Copy link
Contributor

The PR is very large. I wonder if maintainers are more likely to accept it if we split the PR:

  • A first PR that does the safe refactor-only changes (pass IConfigurationService around).
  • A 2nd PR that replaces the constants with the computed sizes.

If the first PR is merged and the 2nd PR is small enough, my team at Google can help test it and iterate on it.

@ElijahPepe
Copy link
Author

@laurentlb I'm happy to split this PR. How would you like to open up communication for that?

@ElijahPepe
Copy link
Author

I'm going to close this for now while @laurentlb and I work towards splitting this PR up and getting changes made through at a much slower and more deliberate pace.

@ElijahPepe ElijahPepe closed this Jun 1, 2022
@ElijahPepe ElijahPepe deleted the ElijahPepe/workbench-adjustments branch June 1, 2022 16:14
@isidorn
Copy link
Contributor

isidorn commented Jun 8, 2022

@ElijahPepe @laurentlb thanks for doing that. Please ping me on any new PRs you open.

@github-actions github-actions bot locked and limited conversation to collaborators Jul 17, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Allow to change the font size and font of the workbench
5 participants