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

feat(terminal): change font size and copy/paste using key shortcuts #612

Merged

Conversation

zackypick
Copy link
Contributor

Closes #531

Description

  • Added ability to increase/decrease the terminal's font size using keyboard shortcuts CTRL(+)+/- and CTRL(+)0 to reset font size
  • Added copy/paste shortcuts using CTRL+SHIFT+(c/v)
  • Fixed compilation issues relating to easy-peasy

Steps to Test

  1. Create / start a network
  2. Launch a terminal on any LN node
  3. Press CTRL(+)+ to increase font size, repeat to further increase until max allowed size
  4. Press CTRL(+)- to decrease font size, repeat to further increase until min allowed size
  5. Press CTRL(+)0 reset terminal's font size
  6. Select some text and press CTRL(+)SHIFT(+)c to copy text to clipboard
  7. Press CTRL(+)SHIFT(+)v to paste text from clipboard

Screenshots

image

@jamaljsr
Copy link
Owner

Thanks for the PR. I'll try to review this later today or tomorrow at the latest.

@jamaljsr
Copy link
Owner

Can you rebase this on master. I noticed that there are a lot of dependency version downgrades in the package.json file.

@zackypick
Copy link
Contributor Author

zackypick commented Oct 28, 2022

No problem

@codecov-commenter
Copy link

codecov-commenter commented Oct 29, 2022

Codecov Report

Base: 100.00% // Head: 100.00% // No change to project coverage 👍

Coverage data is based on head (05bd5b3) compared to base (70f8cfb).
Patch has no changes to coverable lines.

Additional details and impacted files
@@            Coverage Diff            @@
##            master      #612   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files          116       116           
  Lines         3413      3414    +1     
  Branches       612       612           
=========================================
+ Hits          3413      3414    +1     
Impacted Files Coverage Δ
src/components/common/StatusBadge.tsx 100.00% <0.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

Thanks so much for the contribution. I left my feedback below.

@zackypick zackypick requested a review from jamaljsr October 31, 2022 10:56
Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

Thanks for the updates. I tested this and it works great functionally.

The only thing left to do is regarding the diff. Can you please perform the following before merge?

  1. Rebase on the master branch
  2. Revert the changes to package.json, yarn.lock, and tsconfig.json since these do not need to change for this feature
  3. Squash your commits down to a single one. It's not necessary to have multiple commits for a single file change

Feel free to reach out if you're having any trouble with these.

@zackypick zackypick force-pushed the feature/terminal-shortcuts-and-font-size branch from 0960d57 to 8ba299f Compare November 3, 2022 07:12
@zackypick
Copy link
Contributor Author

Done, squashed into a single commit. Hope it can now be merged

@jamaljsr
Copy link
Owner

jamaljsr commented Nov 3, 2022

The diff looks great. I'm going to do a final sanity check on Mac/Win/Linux before merging. Hopefully will get that done tonight or tomorrow.

@jamaljsr
Copy link
Owner

jamaljsr commented Nov 5, 2022

Hey, so sorry but I just realized the Polar asciiart is now out of alignment. It looks like you removed the whitespace on line 45. Can you fix this please?

image

@jamaljsr
Copy link
Owner

jamaljsr commented Nov 5, 2022

BTW: I tested on all OS's so this is good to go once the whitespace is fixed.

@zackypick zackypick force-pushed the feature/terminal-shortcuts-and-font-size branch from 8ba299f to 05bd5b3 Compare November 6, 2022 06:27
@zackypick
Copy link
Contributor Author

No problem, fixed. For some reason, VSCode automatically removed this trailing newline. Anyway hope everything is okay now.

Copy link
Owner

@jamaljsr jamaljsr left a comment

Choose a reason for hiding this comment

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

This looks good to go. 👍 Thanks for your contribution.

@jamaljsr jamaljsr merged commit c4f0ed8 into jamaljsr:master Nov 16, 2022
@zackypick
Copy link
Contributor Author

💯

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.

Feature Request: Add ability to resize text in xterm.js
3 participants