Skip to content

Conversation

@luah5
Copy link
Member

@luah5 luah5 commented Mar 28, 2023

Description

this PR further builds on #1191 by adding the text-zoom feature to the terminal (to mimic xcode) and puts a small fix in TerminalEmulatorView.swift (that is related to text-zoom). I'm offline for tonight, but maintainers have edit access

Related Issues

Checklist

  • I read and understood the contributing guide as well as the code of conduct
  • The issues this PR addresses are related to each other
  • My changes generate no new warnings
  • My code builds and runs on my machine
  • My changes are all related to the related issue above
  • I documented my code

Screenshots

shot.1.mov

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 28, 2023

Can you explain how this is working? Does it require that the user to focus the terminal to zoom in or out when pressing the keyboard shortcut? Will it happen if I don't have focus of the terminal (I might have the editor focused, what happens when I press it then?).

Should the editor and terminal share font and font size? Should we include an item in the terminal font picker for "Editor font" so that when doing a cmd+/- we can zoom in and out both at once? If this is not selected, I'd imagine either the editor or terminal would need to be in focus in order for the key command to apply to it. Thoughts?

@luah5
Copy link
Member Author

luah5 commented Mar 29, 2023

Can you explain how this is working?

increases the font size of the terminal and editor

Does it require that the user to focus the terminal to zoom in or out when pressing the keyboard shortcut? Will it happen if I don't have focus of the terminal (I might have the editor focused, what happens when I press it then?).

even if an editor is closed, the font size will increase, this is not behaviour that we want, i'll change that promptly using .disabled and another if statement. (good catch @austincondiff!)

Should the editor and terminal share font and font size? Should we include an item in the terminal font picker for "Editor font" so that when doing a cmd+/- we can zoom in and out both at once? If this is not selected, I'd imagine either the editor or terminal would need to be in focus in order for the key command to apply to it. Thoughts?

well, it depends what you want, if we want to mimic Xcode behaviour this can be merged straightaway (after the minor fix), but if you prefer the VSCode behaviour (have to focus the terminal to zoom in/out + mentioned preference) i'll add it. personally i prefer the Xcode behaviour as i don't need to focus the terminal to zoom it in, but it needs to be open.

EDIT: I just realised that even if the xcode terminal isn't open it increases/decreases the font size.

Copy link
Contributor

@matthijseikelenboom matthijseikelenboom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no way to reset the zoom

@luah5
Copy link
Member Author

luah5 commented Mar 29, 2023

There is no way to reset the zoom

what do you mean by reset? do you mean that there should be a keybinding/WindowCommand that resets the font size of both the editor and terminal back to 12? i don't think it's necessary as you can just set it back to normal in the preferences.

@matthijseikelenboom
Copy link
Contributor

Well this a weird implantation now that I look at it. Zooming doesn't mean altering the fontsize, it means applying a multiplier to the fontsize. If it should alter the fontsize, then fine, but don't call it zooming. Personally I think using the trackpad to zoom with pinching is nice.

Referring to reset:
image

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 29, 2023

Increasing and decreasing the font size is how it works in Xcode and VS Code.

We can do pinch-to-zoom, but it has to be super smooth. Perhaps we zoom (literally, not change the font size) while pinching to zoom, then after the gesture ends, reset the zoom and adjust the font size to the same visual size as the zoom level which would wrap the text as expected.

@luah5
Copy link
Member Author

luah5 commented Mar 29, 2023

Increasing and decreasing the font size is how it works in Xcode and VS Code.

We can do pinch-to-zoom, but it has to be super smooth. Perhaps we zoom (literally, not change the font size) while pinching to zoom, then after the gesture ends, reset the zoom and adjust the font size to the same visual size as the zoom level which would wrap the text as expected.

i've looked into this and seems more of a bigger feature rather than something i can easily add to this PR.

@austincondiff
Copy link
Collaborator

austincondiff commented Mar 29, 2023

@luah5 Yes, this is way out of scope for this PR. Created issue #1210.

@austincondiff austincondiff self-requested a review March 29, 2023 19:16
@austincondiff austincondiff merged commit dd8fce2 into CodeEditApp:main Mar 29, 2023
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.

3 participants