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

Make copy button icon permanent and strip its white space #25

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

cjsha
Copy link

@cjsha cjsha commented Jan 22, 2025

This PR does two things: makes copy icon appear permanently (instead of only appearing when hovering over the workflow) and strips white space from the copy button. I'm not sure if y'all want this, but I think it's worth considering. We will probably make these additions in our files in the open ephys docs if you don't here.

Make copy button permanent

image

Currently the copy button icon only appears when the cursor hovers over the workflow container. I propose that the copy button appears permanently. This is meant to address the first item in the bullet point of this issue. This accomplished by 3e90c1a.

Strip white space from copy button

image     -->     Screenshot 2025-01-21 215509

The copy button clickable area extends from the clipboard button to the left. Any whitespace contained by an HTML <pre> tag is rendered. Not sure if this was intentional. I propose removing that white space. This is accomplished by 7a2580b.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@glopesdev
Copy link
Member

@cjsha thanks for the contribution. I think this could be split into two different PRs, one visual and the other functional.

Let's discuss this at the next dev club. The reason why we kept the copy button behavior on hover is for consistency with all the other code blocks, e.g. python / c#. Not sure if your PR changes the behavior also for those boxes.

As for the visual change, again as long as the change is to improve consistency with the other blocks I would be happy with it.

@cjsha
Copy link
Author

cjsha commented Feb 3, 2025

3e90c1a changes behavior of the code block and is probably inconsistent with how other code blocks work.
7a2580b basically just changes the clickable area for the copy button. This might be important, for example, if you want to put a button next to it (like a download button), but I suppose that's also inconsistent with other code blocks which I feel like usually have clipboard-copy buttons and no download buttons.

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