Add copy button to snippet
What does this MR do and why?
Add copy button to embedded snippets for parity of actions of a GitLab snippet when used in product or on third party website.
Relates to #336307 (closed)
- Add functionality to copy to clipboard by adding a button to embedded snippets
- Update icons to reflect the thinner icons
Screenshots or screen recordings
Current | Change |
---|---|
![]() |
![]() |
How to set up and validate locally
- Run GDK
- Create a snippet in a local project
- Click
Embed
and copy the script- Example:
<script src="http://127.0.0.1:3000/flightjs/Flight/-/snippets/21.js"></script>
- Example:
- Create a new HTML page and call it
index.html
somewhere on your computer with the following code inside
<!DOCTYPE html>
<html lang="en">
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Document</title>
</head>
<body>
<script src="http://127.0.0.1:3000/flightjs/Flight/-/snippets/21.js"></script>
</body>
</html>
- Open the webpage in a browser
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Merge request reports
Activity
assigned to @mle
Suggested Reviewers (beta)
The individuals below may be good candidates to participate in the review based on various factors.
You can use slash commands in comments to quickly assign
/assign_reviewer @user1
.Suggested Reviewers @pslaughter
,@rspeicher
,@dmishunov
,@dbalexandre
,@rymai
If you do not believe these suggestions are useful, please apply the label Bad Suggested Reviewer. You can also provide feedback for this feature on this issue:
https://gitlab.com/gitlab-org/gitlab/-/issues/357923
.Automatically generated by Suggested Reviewers Bot - an experimental ML-based recommendation engine created by ~"group::applied ml".
Edited by GitLab Reviewer-Recommender Bot3 Warnings 21a3b878: Commits that change 30 or more lines across at least 3 files should describe these changes in the commit body. For more information, take a look at our Commit message guidelines. featureaddition and featureenhancement merge requests normally have a documentation change. Consider adding a documentation update or confirming the documentation plan with the Technical Writer counterpart.
For more information, see:
- The Handbook page on merge request types.
- The definition of done documentation.
This merge request does not refer to an existing milestone. 1 Message CHANGELOG missing: If you want to create a changelog entry for GitLab FOSS, add the
Changelog
trailer to the commit message you want to add to the changelog.If you want to create a changelog entry for GitLab EE, also add the
EE: true
trailer to your commit message.If this merge request doesn't need a CHANGELOG entry, feel free to ignore this message.
If needed, you can retry the
danger-review
job that generated this comment.Reviewer roulette
Changes that require review have been detected!
Please refer to the table below for assigning reviewers and maintainers suggested by Danger in the specified category:
Category Reviewer Maintainer backend Minahil Nichols ( @minahilnichols
) (UTC-4, 14 hours behind@mle
)Kerri Miller ( @kerrizor
) (UTC-7, 17 hours behind@mle
)frontend Miranda Fluharty ( @mfluharty
) (UTC-6, 16 hours behind@mle
)Mike Greiling ( @mikegreiling
) (UTC-5, 15 hours behind@mle
)To spread load more evenly across eligible reviewers, Danger has picked a candidate for each review slot, based on their timezone. Feel free to override these selections if you think someone else would be better-suited or use the GitLab Review Workload Dashboard to find other available reviewers.
To read more on how to use the reviewer roulette, please take a look at the Engineering workflow and code review guidelines. Please consider assigning a reviewer or maintainer who is a domain expert in the area of the merge request.
Once you've decided who will review this merge request, assign them as a reviewer! Danger does not automatically notify them for you.
Generated by
Danger- Resolved by Michael Le
@oregand I need some assistance trying to figure out how to do the following things to complete this MR. If you could help direct me to someone of context of this functionality that would be great.
- How do I implement "copy to clipboard" in Ruby?
- I started a function here !87677 (diffs) in
snippets_helper.rb
but I am not sure of how to get context of file and how to fill the clipboard
- The icon of the button is not correct.
- I don't know how the icon loading works. I am assuming it uses
app/helpers/icons_helper.rb
but it appearscopy-to-clipboard
is not there.
Allure report
allure-report-publisher
generated test report!review-qa-blocking:
test report for de1c8412expand test summary
+-------------------------------------------------------------------+ | suites summary | +----------------------+--------+--------+---------+-------+--------+ | | passed | failed | skipped | flaky | result | +----------------------+--------+--------+---------+-------+--------+ | Plan | 41 | 0 | 1 | 41 | ❗ | | Create | 22 | 0 | 3 | 23 | ❗ | | Manage | 35 | 0 | 3 | 38 | ❗ | | Verify | 12 | 0 | 1 | 12 | ❗ | | Configure | 0 | 0 | 1 | 0 | ➖ | | Version sanity check | 0 | 0 | 1 | 0 | ➖ | | Package | 0 | 0 | 1 | 0 | ➖ | | Protect | 2 | 0 | 0 | 2 | ❗ | +----------------------+--------+--------+---------+-------+--------+ | Total | 112 | 0 | 11 | 116 | ❗ | +----------------------+--------+--------+---------+-------+--------+
- Resolved by Michael Le
Should we be moving forward with this merge request?
To make the copy to clipboard work I am using ClipboardJS. We are using this library in other parts of the application but through existing helper files like:
- app/assets/javascripts/behaviors/copy_to_clipboard.js
- app/helpers/button_helper.rb
There is currently NO JavaScript as part of the embed snippet code.
The snippet delivered on a page via a Ruby router that renders this information
app/views/shared/snippets/show.js.haml
The HTML and CSS to give the embed the correct layout and functionality are delivered as part of page. The existing functionality of viewing the raw file or viewing the source are done with links.By introducing the "copy to clipboard" functionality I had to introduce a third party JavaScript library on the page (Ln 1) because this functionality is not possible using vanilla JavaScript.
The concern I have is that by loading a snippet on the page, page load is taking an extra hit by loading a 3rd party JavaScript library.
Is there a way to work around this dependency of a 3rd party library?
Bundle size analysis [beta]
This compares changes in bundle size for entry points between the commits 12cc4ea4 and de1c8412
Special assetsEntrypoint / Name Size before Size after Diff Diff in percent average 3.46 MB 3.46 MB - 0.0 % mainChunk 1.93 MB 1.93 MB - 0.0 %
Note: We do not have exact data for 12cc4ea4. So we have used data from: 97d29dc2.
The intended commit has no webpack pipeline, so we chose the last commit with one before it.Please look at the full report for more details
Read more about how this report works.
Generated by
Dangeradded 2 commits
- Resolved by Michael Le
@mle - please add typebug typefeature, typemaintenance or a subtype label to this merge request.- typebug: Defects in shipped code and fixes for those defects. This includes all the bug types (availability, performance, security vulnerability, mobile, etc.)
- typefeature: Effort to deliver new features, feature changes & improvements. This includes all changes as part of new product requirements like application limits.
- typemaintenance: Up-keeping efforts & catch-up corrective improvements that are not Features nor Bugs. This includes restructuring for long-term maintainability, stability, reducing technical debt, improving the contributor experience, or upgrading dependencies.
See the handbook for more guidance on classifying.
added featureaddition typefeature labels
added groupeditor [DEPRECATED] + 1 deleted label
requested review from @dfrazao-gitlab and @sheldonled
- Resolved by Michael Le
Hi @dfrazao-gitlab could you please provide a backend review?
- Resolved by Kerri Miller
Hi @sheldonled could you please provide a frontend review?
requested review from @iamphill and removed review request for @sheldonled
@sheldonled
, thanks for approving this merge request.This is the first time the merge request is approved. To ensure full test coverage, a new pipeline has been started.
For more info, please refer to the following links:
removed review request for @iamphill
requested review from @kerrizor and removed review request for @dfrazao-gitlab
- Resolved by Anastasia McDonald
@mle Just a heads up that
review
failures are there due to the branch being old and rebasing should fix it.
added 2376 commits
-
0a1d5d95...9c52a872 - 2375 commits from branch
master
- 8a215b09 - Add copy button to snippet
-
0a1d5d95...9c52a872 - 2375 commits from branch
added 212 commits
-
8a215b09...3abf3aa8 - 211 commits from branch
master
- 0d52cdb2 - Add copy button to snippet
-
8a215b09...3abf3aa8 - 211 commits from branch
Setting label(s) devopscreate sectiondev based on ~"group::editor".
added devopscreate sectiondev labels
mentioned in merge request !85823 (closed)
- Resolved by Michael Le
- Resolved by Kerri Miller
@mle just a small "naming things is hard" nit - LMKWYT
added 384 commits
-
a6267f4b...98908d43 - 381 commits from branch
master
- f32b5184 - Add copy button to snippet
- 05b1f2b9 - Improve the naming of the button function
- e051408a - Update function name in spec
Toggle commit list-
a6267f4b...98908d43 - 381 commits from branch
- Resolved by Kerri Miller
enabled an automatic merge when the pipeline for 01b39276 succeeds
- Resolved by Kerri Miller
sigh
broken master..
/rebase
and rerunning pipelines
added 166 commits
-
4a850cc4...8c4b2694 - 162 commits from branch
master
- 3c18a3b2 - Add copy button to snippet
- 4b90c86b - Improve the naming of the button function
- bb21232a - Update function name in spec
- acef9732 - Call renamed method in HAML
Toggle commit list-
4a850cc4...8c4b2694 - 162 commits from branch
enabled an automatic merge when the pipeline for ddaab27d succeeds
added 40 commits
-
acef9732...12cc4ea4 - 36 commits from branch
master
- 21a3b878 - Add copy button to snippet
- 935b4825 - Improve the naming of the button function
- de0becee - Update function name in spec
- de1c8412 - Call renamed method in HAML
Toggle commit list-
acef9732...12cc4ea4 - 36 commits from branch
mentioned in commit f5f43909
added workflowstaging-canary label
added workflowcanary label and removed workflowstaging-canary label
mentioned in merge request !89072 (closed)
added workflowstaging label and removed workflowcanary label
added workflowproduction label and removed workflowstaging label
mentioned in merge request !89367 (closed)
FYI @oregand I ran into a bug where using the navigator.clipboard API resulted in an error when I checked on a third party website https://www.embeddeddevops.com/post/testing-elf-files
Uncaught (in promise) DOMException: The Clipboard API has been blocked because of a permissions policy applied to the current document. See https://goo.gl/EuHzyv for more details.
I have created a patch to fix this !89367 (closed)
Thank you! I will hold off on more work for Update Embedded Snippets UI - Icon && Buttons (!89072 - closed) until this fix is in
mentioned in issue #336307 (closed)
added releasedpublished label
added releasedcandidate label and removed releasedpublished label
added Category:Source Code Management snippets labels and removed 1 deleted label