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

CopySnippet: convert to TypeScript (HDS-2689) #2110

Merged
merged 6 commits into from
May 30, 2024
Merged

CopySnippet: convert to TypeScript (HDS-2689) #2110

merged 6 commits into from
May 30, 2024

Conversation

KristinLBradley
Copy link
Contributor

@KristinLBradley KristinLBradley commented May 16, 2024 •
edited

📌 Summary

If merged, this PR converts the CopySnippet to TypeScript.

The CopySnippet depends upon:

🔗 External links

👀 Component checklist

  • [ ] Percy was checked for any visual regression
  • A changelog entry was added via Changesets if needed (see templates here)

💬 Please consider using conventional comments when reviewing this PR.

@vercel Vercel
Copy link

vercel bot commented May 16, 2024 •
edited

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated (UTC)
hds-showcase ✅ Ready (Inspect) Visit Preview May 23, 2024 7:01pm
hds-website ✅ Ready (Inspect) Visit Preview May 23, 2024 7:01pm

@KristinLBradley KristinLBradley requested a review from a team May 16, 2024 23:13
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from 70511b2 to daede28 Compare May 16, 2024 23:24
@didoo didoo mentioned this pull request May 17, 2024
Merged
1 task
didoo
didoo reviewed May 17, 2024
View reviewed changes
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

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

Left a few comments (similar to the CopyButton)

packages/components/src/components/hds/copy/snippet/index.ts Show resolved Hide resolved
packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
@KristinLBradley KristinLBradley force-pushed the hds-2688-copy-button-ts-conversion branch from 834131f to 03b02a3 Compare May 17, 2024 20:45
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from ad1086e to 787300d Compare May 17, 2024 20:47
@KristinLBradley KristinLBradley marked this pull request as draft May 20, 2024 20:47
@KristinLBradley KristinLBradley mentioned this pull request May 20, 2024
Merged
1 task
@KristinLBradley KristinLBradley force-pushed the hds-2688-copy-button-ts-conversion branch from 62a1cba to 6d0f2fe Compare May 20, 2024 20:53
@KristinLBradley KristinLBradley changed the base branch from hds-2688-copy-button-ts-conversion to hds-3344-clipboard-ts-conversion May 20, 2024 21:13
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from 787300d to 80e7e0b Compare May 20, 2024 21:17
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from 80e7e0b to 74a5658 Compare May 20, 2024 22:47
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from 74a5658 to 85d4385 Compare May 20, 2024 23:37
@KristinLBradley KristinLBradley force-pushed the hds-2689-copy-snippet-ts-conversion branch from 85d4385 to 57927f8 Compare May 21, 2024 00:04
@KristinLBradley KristinLBradley marked this pull request as ready for review May 21, 2024 22:19
@KristinLBradley KristinLBradley requested review from a team and alex-ju May 21, 2024 22:20
didoo
didoo reviewed May 22, 2024
View reviewed changes
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

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

The code is good and could be approved, but before I want to discuss with @alex-ju the issue with the bigint support.

packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
packages/components/src/components/hds/copy/snippet/index.ts Outdated Show resolved Hide resolved
packages/components/src/template-registry.ts Outdated Show resolved Hide resolved
didoo
didoo approved these changes May 22, 2024
View reviewed changes
Copy link
Contributor

@didoo didoo left a comment

Choose a reason for hiding this comment

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

Apart from the suggestions, everything else is OK 👍

@KristinLBradley KristinLBradley force-pushed the hds-2688-copy-button-ts-conversion branch from a89bb3e to 72e40ea Compare May 23, 2024 17:33
@KristinLBradley
Copy link
Contributor Author

KristinLBradley commented May 23, 2024 •
edited

@didoo I've been trying to rebase this branch onto hds-2688-copy-button-ts-conversion but keep getting conflicts and when I try to fix the conflicts and continue rebasing the conflicts re-occur.

@didoo didoo force-pushed the hds-2689-copy-snippet-ts-conversion branch from c6056f6 to db6a6f7 Compare May 23, 2024 18:50
@didoo didoo force-pushed the hds-2689-copy-snippet-ts-conversion branch from ff6c5e5 to 74d3466 Compare May 23, 2024 18:54
@didoo
Copy link
Contributor

didoo commented May 24, 2024 •
edited

@didoo I've been trying to rebase this branch onto hds-2688-copy-button-ts-conversion but keep getting conflicts and when I try to fix the conflicts and continue rebasing the conflicts re-occur.

For future reference: this was fixed yesterday with a rebase of the branch hds-2689-copy-snippet-ts-conversion on hds-2688-copy-button-ts-conversion (and in the process of fixing conflicts I've merged and/or cleaned up a few commits) and then messaged Kristin to let her know.

@KristinLBradley KristinLBradley merged commit 2188273 into hds-2688-copy-button-ts-conversion May 30, 2024
16 checks passed
@KristinLBradley KristinLBradley deleted the hds-2689-copy-snippet-ts-conversion branch May 30, 2024 21:28
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@didoo didoo didoo approved these changes

@alex-ju alex-ju alex-ju approved these changes

Assignees
No one assigned
Labels
packages/components typescript
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@KristinLBradley @didoo @alex-ju @hashibot-hds