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

fix: copy logs text on Linux & Windows. Fixes #557 #558

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

Adrien-D
Copy link
Member

Fixes #557

Inspired from xtermjs/xterm.js#2478 (comment) this attachCustomKeyEventHandler on the terminal catch the Ctrl+C event, and if there is a current text selection, we force the browser to copy that text.

@JPZ13
Copy link
Member

JPZ13 commented Jun 3, 2024

@agilgur5 @terrytangyuan - who would be the best to ask for a review on this and other UI PRs? We have a couple UI PRs coming down the pipeline

@terrytangyuan
Copy link
Member

terrytangyuan commented Jun 3, 2024

I think @argoproj/argo-ui-maintainers can help review/merge this

@agilgur5
Copy link
Contributor

agilgur5 commented Jun 6, 2024

Followed up on Slack.

As per there, prior UI maintainers unfortunately do not respond and the GH team name doesn't work anymore either 😕 See also #453 et al.

I've been reviewing various code here since I've had Approver permissions in this repo, and have asked Crenshaw, Terry, and Remington for review before. Crenshaw has responded after I've pinged him or the formerly named #argo-contributors channel several times. Remington responded a bit in the past, but not recently. In recent times, I've asked Isitha for review too since we both have Approver permissions now and Isitha works a bit on UI in Workflows.

As I'm the only active UI SME on Workflows, it would be good if I reviewed more substantial changes. If I'm not available or it's a smaller change, I think others could respond.

I was also waiting for Adrien to respond to code review I'd done last week before adding more; he did so earlier today.

@agilgur5 agilgur5 self-assigned this Jun 6, 2024
@agilgur5 agilgur5 changed the title fix: copy logs text on windows. Fixes #557 fix: copy logs text on Linux & Windows. Fixes #557 Jun 6, 2024
Signed-off-by: Adrien Delannoy <[email protected]>
@Adrien-D Adrien-D requested a review from agilgur5 June 6, 2024 13:35
@agilgur5 agilgur5 enabled auto-merge (squash) June 6, 2024 16:35
Copy link
Contributor

@agilgur5 agilgur5 left a comment

Choose a reason for hiding this comment

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

LGTM

@agilgur5 agilgur5 merged commit 0bce165 into argoproj:master Jun 6, 2024
5 checks passed
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.

Copy from logs using Ctrl-C doesn't work on Linux & Windows
4 participants