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

Search in header should not wrap anchor around button semantically #2088

Closed
BorghildSelle opened this issue Jan 30, 2024 · 2 comments
Closed
Assignees
Labels
🐛 bug Something isn't working

Comments

@BorghildSelle
Copy link
Contributor

BorghildSelle commented Jan 30, 2024

A Next link is wrapped around Button component to seemingly accomplish a "button"-look for the link.
But semantically a anchor tag should not contain an button element. Button should be used with onClick event.

This results in an blue outline for the anchor when tabbing to the search, instead of the focus design meant for button( that you get when tabbing one more time into the button element)

Should implement as suggested in eds;

A special note about nextLink: The href type from this component conflicts with the native href type, therefore it is recommended to use the legacy behavior in this case, or alternatively ts-ignore.

import NextLink from 'next/link'
import { Button } from '@equinor/eds-core-react'

<NextLink href="/about" passHref legacyBehavior>
  <Button as="a">About</Button>
</NextLink>

@BorghildSelle BorghildSelle added the 🐛 bug Something isn't working label Jan 30, 2024
@BorghildSelle
Copy link
Contributor Author

This issue also applies to the portrait cards, should maybe run a search for nextlink wrappers with button look

@BorghildSelle BorghildSelle changed the title Search in header should not wrap anchor around button sematically Search in header should not wrap anchor around button semantically Jan 30, 2024
@BorghildSelle BorghildSelle self-assigned this Jan 31, 2024
@BorghildSelle BorghildSelle added 🚀 ready to deploy Use this if issue is ready to be deployed and removed 🚀 ready to deploy Use this if issue is ready to be deployed labels Feb 13, 2024
@meols
Copy link
Collaborator

meols commented Feb 19, 2024

Thanks @BorghildSelle - this is approved 😄

BorghildSelle added a commit that referenced this issue Feb 21, 2024
* 🎨 improve buttonlinks semantics

* 🐛 move to new branch to fix commit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🐛 bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants