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

Issue with AnchorNav setting the primary action to be styled as a primary button #826

Closed
stamat opened this issue Nov 20, 2024 · 7 comments · Fixed by #859
Closed

Issue with AnchorNav setting the primary action to be styled as a primary button #826

stamat opened this issue Nov 20, 2024 · 7 comments · Fixed by #859
Labels

Comments

@stamat
Copy link
Contributor

stamat commented Nov 20, 2024

I wanted to style the primary action of the AnchorNav.Action as the primary button. It defaults to secondary styles. In order to use the primary button styles we need to have two buttons. I couldn't find anything in the docs. Maybe this is the intended behavior?

Anyway I came up with a hack for this... Is there a non hacky way to achieve this

Image

@rezrah rezrah added the brand label Dec 9, 2024
@rezrah
Copy link
Collaborator

rezrah commented Dec 9, 2024

Maybe this is the intended behavior?

👋 @stamat that's correct. It's mentioned in our docs here.

Image

I recall the reason for going with secondary there was to not overpower the adjacent links.

@stamat
Copy link
Contributor Author

stamat commented Dec 9, 2024

@rezrah you think we should force the outline button if there is only one CTA? I mean that's ok with me, but not what we got in the designs... I personally prefer the primary action, and then if we have another one we use the secondary outline for it... But that's just my preference...

@rezrah
Copy link
Collaborator

rezrah commented Dec 9, 2024

Looping in Site FR @natalie-iv

@rezrah
Copy link
Collaborator

rezrah commented Dec 10, 2024

@stamat It turns out that you can forward the primary variant to the action 👇

<AnchorNav>
  <AnchorNav.Action variant="primary">Sign up</AnchorNav.Action>
</AnchorNav>

Image

We're only providing sensible defaults in the component, not enforcing them. This means you can forward all the usual Button props to that component. This should meet the requirements of your request.

FWIW we're going to make a few changes to AnchorNav following a convo with Site yesterday:

  • Make buttons smaller
  • Enforce primer/secondary pairing of two primaries are used

@rezrah rezrah closed this as completed Dec 10, 2024
@stamat
Copy link
Contributor Author

stamat commented Dec 10, 2024

@rezrah ah TMYK! Thanks legend! ✨

@stamat
Copy link
Contributor Author

stamat commented Dec 10, 2024

@rezrah eeeeeerrrrrr.... I think I tried variant before...
Image

@rezrah
Copy link
Collaborator

rezrah commented Dec 11, 2024

Thanks for trying it @stamat, you're absolutely right, sorry for not noticing that. We're missing the type definitions to make this work in TS environments. I've opened a PR here that fixes this (and other things). Should land soon 🤞

@rezrah rezrah reopened this Dec 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants