search

argoproj / argo-ui

Argoproj shared React components
Apache License 2.0
220 stars 178 forks source link

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

Closed Adrien-D closed 3 months ago

Adrien-D commented 3 months ago

Fixes #557

Inspired from https://github.com/xtermjs/xterm.js/issues/2478#issuecomment-1002371588 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 commented 3 months ago

@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 commented 3 months ago

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

agilgur5 commented 3 months ago

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.