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

Make ambassador cards shiny #161

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

Conversation

farrenr
Copy link

@farrenr farrenr commented Apr 21, 2024

Resolves #67

  • Used react-parallax-tilt to create tilt and shine effects on ambassador cards.
  • Added respective user setting to disable card effects.

@farrenr farrenr marked this pull request as ready for review April 21, 2024 20:11
@MattIPv4 MattIPv4 self-requested a review April 22, 2024 00:31
Copy link
Member

@MattIPv4 MattIPv4 left a comment

Choose a reason for hiding this comment

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

👋 Looking great, but there are a few issues I noticed:

src/components/ambassadorCard/AmbassadorCard.tsx Outdated Show resolved Hide resolved
src/components/ambassadorCard/AmbassadorCard.tsx Outdated Show resolved Hide resolved
src/components/ambassadorCard/AmbassadorCard.tsx Outdated Show resolved Hide resolved
src/components/ambassadorCard/AmbassadorCard.tsx Outdated Show resolved Hide resolved
@farrenr farrenr requested a review from MattIPv4 April 23, 2024 17:08
Copy link
Member

Choose a reason for hiding this comment

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

The only remaining issue looks to be that the tooltip on conservation status is now showing in the middle of nowhere?

Copy link
Author

Choose a reason for hiding this comment

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

Good spot! Let me take a look

Copy link
Member

Choose a reason for hiding this comment

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

👋 @farrenr checking in if you had a chance to look at this?

Copy link
Author

Choose a reason for hiding this comment

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

Hey! Apologies on taking a while with this, I've been a bit stuck on the last issue with the tooltip but I'm looking at it today

Co-authored-by: Matt Cowley <me@mattcowley.co.uk>
@pjeweb
Copy link
Member

pjeweb commented May 11, 2024

I think it looks very cool, but I think it could be a bit smoother. The problem with it resetting on the edges and flickering seems to be inherent to the library: https://imgur.com/6nkHLFe

I think with increasing the transition speed to like 5s or so it gets a lot smoother overall, especially when leaving the card as it does not "snap" back to quickly.

I also think the glare could be less or the font contrast gets too low. Its already not the best contrast. Maybe reducing the opacity to 30% could work.

Another thing i noticed, we do have a checkbox to disable the effect. Is there any way to also disable it when prefers-reduced-motion is active? Not sure how to handle the two - maybe just make the checkbox readonly with a note ("disabled due to browser preferences") or hide the checkbox completely?

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.

Make ambassador card shiny
3 participants