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

Small buttons update #1608

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

Small buttons update #1608

wants to merge 4 commits into from

Conversation

SvMak
Copy link
Contributor

@SvMak SvMak commented Dec 14, 2024

  • use cva(class-variance-authority) for variants;
  • made all buttons full rounded.

Copy link
Collaborator

@thilobillerbeck thilobillerbeck left a comment

Choose a reason for hiding this comment

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

Introducing class-variance-authority is definitely a good idea, though I have two small nitpicks. :)

import Link from '@/components/util/Link.astro';

export const buttonVariants = cva(
'font-bold text-white drop-shadow transition-colors rounded-full text-center',
Copy link
Collaborator

@thilobillerbeck thilobillerbeck Dec 24, 2024

Choose a reason for hiding this comment

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

Fully rounded corners are an interesting design change, though I'm not really a fan of it, this comes especially into play when seeing how buttons behave inside cards as seen on the learn site. The fully rounded corners do not really seem to fit into the more boxy card shape, while the old design aligns more with its parent design and at least in my opinion looks more coherent.

Comment on lines +12 to +15
orange: 'bg-nix-orange hover:bg-nix-orange-hover',
lightblue: 'bg-nix-blue-light hover:bg-nix-blue-light-hover',
semidarkblue: 'bg-nix-blue-dark hover:bg-nix-blue-dark-hover',
green: 'bg-nix-green hover:bg-nix-green-hover',
Copy link
Collaborator

Choose a reason for hiding this comment

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

In my testing I couldn't really tell how the hover state applied, am I missing something or is it just to subtile? I'd guess the mentioned colors may need to be defined first. :D

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