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

fix: logo only focusable if href provided #2207

Merged
merged 4 commits into from
Nov 29, 2023
Merged

Conversation

felix-ico
Copy link
Collaborator

@felix-ico felix-ico commented Nov 16, 2023

fixes #2188

Copy link

netlify bot commented Nov 16, 2023

Deploy Preview for marvelous-moxie-a6e2fe ready!

Name Link
🔨 Latest commit 427a4fe
🔍 Latest deploy log https://app.netlify.com/sites/marvelous-moxie-a6e2fe/deploys/6560b229dde28200081b08e6
😎 Deploy Preview https://deploy-preview-2207--marvelous-moxie-a6e2fe.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link
Collaborator

@acstll acstll left a comment

Choose a reason for hiding this comment

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

Fixes the issue but then the logo takes 2 tab hits to move focus away from the logo 🤪 — I wonder if it's easy to make it just 1 step. (Not straightforward to figure out what exactly has the focus because of the shadow DOM). Also, probably the focusable prop in scale-logo could be used?

Plus, this same could be added in telekom-footer-content.tsx, no?

Copy link
Collaborator

@acstll acstll left a comment

Choose a reason for hiding this comment

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

👌

@felix-ico felix-ico merged commit 63c8657 into main Nov 29, 2023
11 checks passed
@felix-ico felix-ico deleted the fix/focusable-logo branch November 29, 2023 17:30
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.

Telekom Brand Header: Logo is focusable without logoHref prop
2 participants