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.

Sign up for GitHub

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

Jump to bottom

feat: add the ability to copy module import to the clipboard #754

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

feat: add the ability to copy module import to the clipboard #754

wants to merge 5 commits into from

Conversation

hadzhiyski
Copy link

@hadzhiyski hadzhiyski commented Apr 28, 2020 •
edited
Loading

@hadzhiyski hadzhiyski mentioned this pull request Apr 28, 2020
Closed
Splaktar
Splaktar suggested changes Apr 29, 2020
View reviewed changes
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

Thank you for the contribution!

Can you please post a screenshot in the OP?

src/app/shared/doc-viewer/docs-api-module-import/docs-api-module-import.ts Show resolved Hide resolved
src/app/shared/doc-viewer/docs-api-module-import/docs-api-module-import.ts Outdated
@@ -0,0 +1,50 @@
import { Component, Input, AfterViewInit, OnInit } from "@angular/core";
import { MatSnackBar } from "@angular/material/snack-bar";
import { CopierService } from "../../copier/copier.service";
Copy link
Member

Choose a reason for hiding this comment

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

We should probably look at replacing the CopierService with the new https://material.angular.io/cdk/clipboard/overview.

Copy link
Author

Choose a reason for hiding this comment

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

I was wondering about the CopierService, but left it that way for consistency. There are other components which are using it as well, for example the ExampleViewer component. It might be a good idea to switch to Clipboard with separate PR.

@hadzhiyski
Copy link
Author

image

Here is a screenshot of the changes. Ignore the fact that Material autocomplete is importing MatButtonModule. It is just for the test.

@hadzhiyski
Copy link
Author

@Splaktar this PR is ready for review.

Splaktar
Splaktar suggested changes Jun 8, 2020
View reviewed changes
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

Thank you for making those updates. Sorry for the delay in getting to this review.

Something happened with your last update/rebase. I don't think that this change should have 49 changed files and many of them seem to be coming from other commits. Can you please rebase your commit on top of the latest master and force push it up again?

angular.json Outdated Show resolved Hide resolved
src/app/shared/doc-viewer/docs-api-module-import/docs-api-module-import.ts Outdated Show resolved Hide resolved
src/app/shared/doc-viewer/docs-api-module-import/docs-api-module-import.ts Show resolved Hide resolved
@googlebot
Copy link
Collaborator

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link
Collaborator

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link
Collaborator

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@hadzhiyski hadzhiyski requested a review from Splaktar June 15, 2020 10:29
@hadzhiyski
Copy link
Author

Hello @Splaktar,
It has been a while since your last review. From my perspective, everything is covered. Do you expect something else which I might have missed?

Splaktar
Splaktar suggested changes Jul 14, 2020
View reviewed changes
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

I checked this out locally to test it. I also rebased it on the latest master. However, I was unable to get the copy icon to appear or be rendered?

Screen Shot 2020-07-14 at 16 31 52

Screen Shot 2020-07-14 at 16 35 42

@hadzhiyski
Copy link
Author

Hi,
Most probably the reason is this angular/components#19200 . If you test it locally, you should rebuild the docs locally as well, because their PR is still open.

@Splaktar
Copy link
Member

OK, the other PR looks like it's ready to go, hopefully we can merge that soon. Can you please rebase this PR as well?

@google-cla
Copy link

google-cla bot commented May 16, 2021

All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter.

We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only @googlebot I consent. in this pull request.

Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the cla label to yes (if enabled on your project).

ℹ️ Googlers: Go here for more info.

@google-cla google-cla bot added cla: no and removed cla: yes labels May 16, 2021
@hadzhiyski
Copy link
Author

@googlebot I consent.

@hadzhiyski
Copy link
Author

@googlebot I consent

@google-cla google-cla bot added cla: yes and removed cla: no labels Nov 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Splaktar Splaktar Splaktar requested changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
cla: yes feature P4: nice to have pr: in-progress
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add copy button to API references
3 participants
@hadzhiyski @googlebot @Splaktar