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.

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

Tokunbo #91

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

Tokunbo #91

wants to merge 5 commits into from

Conversation

Teea-dev
Copy link
Contributor

@Teea-dev Teea-dev commented Jan 3, 2025

I have read the CONTRIBUTING.md file.

YES

I have run yarn gen-cli to generate the necessary files

YES

What kind of change does this PR introduce?

added three new icons and also fixed the clipboard-check icon

What is the new behavior?

addition of three new icons and a fix to one
Feel free to include screenshots if it includes visual changes.

Demo

Please attach a short video demo of the changes.

lock.icon.mp4
heart.icons.mp4

Additional context

Add any other context or screenshots.

Copy link

vercel bot commented Jan 3, 2025

@Teea-dev is attempting to deploy a commit to the pqoqubbw Team on Vercel.

A member of the Team first needs to authorize it.

@pqoqubbw
Copy link
Owner

pqoqubbw commented Jan 3, 2025

hey, for lock icon it's okay just show open state. also there are a few fixes for lock icon:
svg size cropped
CleanShot 2025-01-03 at 4  12 15@2x

you should move not only body of the lock to the bottom
CleanShot 2025-01-03 at 4  12 24@2x

@pqoqubbw
Copy link
Owner

pqoqubbw commented Jan 3, 2025

regarding hearts:

  • for basic icon, it would be okay just the pulse animation, not pathLength
  • for cracked heart i guess would be cool to show this crack via rotating a little each half of the heart

Comment on lines 33 to 34
width="24"
height="24"
Copy link
Owner

Choose a reason for hiding this comment

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

Suggested change
width="24"
height="24"
width="28"
height="28"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Making changes to these now

className="cursor-pointer select-none p-2 hover:bg-accent rounded-md transition-colors duration-200 flex items-center justify-center"
onMouseEnter={() => controls.start('animate')}
onMouseLeave={() => controls.start('normal')}
style={{ display: 'inline-block' }}
Copy link
Owner

Choose a reason for hiding this comment

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

why do you need that?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

you mean the style right?

Copy link
Owner

Choose a reason for hiding this comment

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

yep

icons/heart.tsx Outdated
className="cursor-pointer select-none p-2 hover:bg-accent rounded-md transition-colors duration-200 flex items-center justify-center"
onMouseEnter={() => controls.start('animate')}
onMouseLeave={() => controls.start('normal')}
style={{ display: 'inline-block' }}
Copy link
Owner

Choose a reason for hiding this comment

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

same here

icons/heart.tsx Outdated
Comment on lines 32 to 33
width="24"
height="24"
Copy link
Owner

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would do

icons/lock.tsx Outdated
Comment on lines 51 to 52
width="24"
height="24"
Copy link
Owner

Choose a reason for hiding this comment

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

same here

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would do

Copy link
Owner

@pqoqubbw pqoqubbw left a comment

Choose a reason for hiding this comment

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

also this changes should not be in the PR
CleanShot 2025-01-03 at 4  18 00@2x
CleanShot 2025-01-03 at 4  17 56@2x

@Teea-dev
Copy link
Contributor Author

Teea-dev commented Jan 3, 2025

also this changes should not be in the PR CleanShot 2025-01-03 at 4  18 00@2x CleanShot 2025-01-03 at 4  17 56@2x

my bad

…vg size and also moved not only the body of the lock to the bottom, changed the heart-crack animation (not soo sure I got it tho) and other changes as requested.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants