Skip to content
This repository has been archived by the owner on Oct 18, 2024. It is now read-only.

feat: added copy feature #175

Merged
merged 14 commits into from
Sep 9, 2024
Merged

feat: added copy feature #175

merged 14 commits into from
Sep 9, 2024

Conversation

Bashamega
Copy link
Member

@Bashamega Bashamega commented Aug 16, 2024

Fixes Issue

closes #173

Changes proposed

Check List (Check all the applicable boxes)

  • My code follows the code style of this project.
  • [] My change requires changes to the documentation.
  • I have updated the documentation accordingly.
  • All new and existing tests passed.
  • This PR does not contain plagiarized content.
  • The title of my pull request is a short description of the requested changes.

Screenshots

image

Note to reviewers

Copy link
Member

@eddiejaoude eddiejaoude 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 👍

Please can you add a screenshot for desktop and mobile view.

@Bashamega Bashamega requested a review from eddiejaoude August 16, 2024 12:04
@eddiejaoude
Copy link
Member

eddiejaoude commented Aug 16, 2024

@Bashamega you requested a review but there are no changes since my last review - did you forget to push?

note: please don't mark inline discussions as resolved, as it makes it more difficult to review

@Bashamega
Copy link
Member Author

@Bashamega you requested a review but there are no changes since my last review - did you forget to push?

note: please don't mark inline discussions as resolved, as it makes it more difficult to review

I am very sorry. I have forgot to push the changes. LOL!

@Bashamega
Copy link
Member Author

Hello @eddiejaoude I have updated the form to use the Input component and made the input component more customizable

@Bashamega Bashamega requested a review from eddiejaoude August 22, 2024 12:36
aria-hidden="true"
className="h-5 w-5 text-gray-400"
className={`h-5 w-5 ${copy ? "text-green-400 " : "text-gray-500 "} `}
Copy link
Member

Choose a reason for hiding this comment

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

Best to be consistent and use classNames function here to concatenate class strings

@Bashamega
Copy link
Member Author

I have fixed the problems :)

@@ -3,11 +3,14 @@
import {
DocumentDuplicateIcon,
CheckBadgeIcon,
Copy link
Member

Choose a reason for hiding this comment

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

I think CheckBadgeIcon is no longer used.

Don't worry about fixing it, I will merge to a temporary branch and fix now, so we can get this feature merged

Copy link
Member

@eddiejaoude eddiejaoude 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 👍

@eddiejaoude eddiejaoude changed the base branch from main to feat-copy September 9, 2024 11:14
@eddiejaoude eddiejaoude merged commit 2a3798b into EddieHubCommunity:feat-copy Sep 9, 2024
2 checks passed
eddiejaoude added a commit that referenced this pull request Sep 9, 2024
* feat: copy badge feature (#175)

* feat: added copy feature

* Switched to asyc

* Removed the console log

* Format fix

* Updated input

* format

* Uses classnames

* Update src/components/forms/Input.js

* Update src/app/account/repo/checks/[id]/form.js

Co-authored-by: Eddie Jaoude <[email protected]>

* Update src/app/account/repo/checks/[id]/form.js

Co-authored-by: Eddie Jaoude <[email protected]>

* patch: class names

* patch: format

* Update src/app/account/repo/checks/[id]/form.js

Co-authored-by: Eddie Jaoude <[email protected]>

* Update src/app/account/repo/checks/[id]/form.js

---------

Co-authored-by: Eddie Jaoude <[email protected]>

* fix: simplified copy

* fix: badge form label

* fix: revert input component

---------

Co-authored-by: Adam Basha <[email protected]>
@Bashamega Bashamega deleted the clipboard branch October 18, 2024 09:10
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEATURE] Add a copy button.
2 participants