Added copy commit SHA1 button in commits list #13852
Conversation
Fixes #13816 Signed-off-by: Vangelis <macvag@gmail.com>
| @@ -56,6 +56,7 @@ | |||
| {{else}} | |||
| </span> | |||
| {{end}} | |||
| <span style="cursor: pointer;vertical-align: middle;" class="clipboard" data-original="{{$.i18n.Tr "repo.commits.copy_sha"}}" data-success="{{$.i18n.Tr "repo.commits.copy_sha_success"}}" data-error="{{$.i18n.Tr "repo.commits.copy_sha_error"}}" data-content="{{$.i18n.Tr "repo.commits.copy_sha"}}" data-variation="inverted tiny" data-clipboard-text="{{.ID}}">{{svg "octicon-clippy"}}</span> | |||
There was a problem hiding this comment.
Please refrain from adding additional inline CSS.
There was a problem hiding this comment.
I made an improvement by removing the inline style and using the existing ones for better consistency.
Also, I added it inside the tag to get stacked better with the other icons.
Because it's inside the tag I had to add onclick="event.preventDefault()".
Is that ok to commit it like that?
There was a problem hiding this comment.
No, inline scripts are also not appreciated. Make it an actual <button> and you won't need it while also being semantically correct.
There was a problem hiding this comment.
Unfortunately, I can't just add a button outside the tag.
For example, If we have it as a button outside the link I must create new styles for that button to fit correctly
If there is also the icon for a signed commit we have the below result with the three dots and also we have to create new styles for the content to fit correctly in the cell and rearrange the content with the proper order.
If we put it inside the link with the same styles that other icons have there, the icons are consistent but we have to prevent the redirection to commit details somehow.
Whatever it must be done I believe that the copy should be consistent with the other copy actions across the site.
GiteaBot
added
the
lgtm/need 2
techknowlogick
added
type/enhancement
Codecov Report
@@ Coverage Diff @@
## master #13852 +/- ##
==========================================
+ Coverage 42.13% 42.24% +0.11%
==========================================
Files 708 708
Lines 77167 77173 +6
==========================================
+ Hits 32514 32605 +91
+ Misses 39306 39201 -105
- Partials 5347 5367 +20
Continue to review full report at Codecov.
|
|
Screenshot please. |
|
I think we first need to rework those shaboxes (which can appear in like 15 different places) to use a shared go template and shared CSS. Maybe then we can add copy functionality that only shows on hover to not further complicate their already complex rendering. Unfortunately this means that unless you're willing to do that rework, I see no future for this PR because doing it in only one place is too inconsistent. |
|
Thought we may be able to make an exception and have the copy button on the commit list after all as there's space. Maye have a separated button in the same height as the shabox (will need some manual CSS). |
|
Maybe today I will found some free time to try fixing that issue with new css |
|
I think you can take the basic button from your first screenshot and downsize it via CSS to match the height of the shabox and it should be fine. Don't even need to color the background. Will likely need to increase the width of the column so it doesn't overflow on signed commits which are wider. |
|
status? @macvag @silverwind |
|
@macvag any time soon? |
|
Sorry for my late reply. I didn't notice the first mention. |
|
This is not ready and needs further work - therefore I'm moving it to 1.16 unless there is someone willing to take this up and fix it. |
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs during the next 2 months. Thank you for your contributions. |
|
We have a better implementation |
Fixes #13816
Signed-off-by: Vangelis macvag@gmail.com