feat(AsideHeader, Logo)!: make actions accessible with keyboard #305
Conversation
kmenshov
requested review from
DarkGenius,
Lunory and
sunduckcow
as code owners
|
Preview is ready. |
|
Playwright Test Component is ready. |
| {logo} | ||
| </a> | ||
| ))} | ||
| {hasWrapper ? wrapper(button, Boolean(compact)) : button} |
There was a problem hiding this comment.
The button is not suitable for all cases. When clicking the button, unnecessary visual effects occur. In addition, there may be uses of wrappers that look at the node markup. In the major version, we plan to improve this component
There was a problem hiding this comment.
So, I've replaced the Button element with a native tag. This removes the clicking visual effects and still keeps the element tabable.
This time, however, I had to retake a couple of screenshots (Footer and MobileFooter). There seems to be no visual difference in Storybook, including Safari, but there is a slight logo shift in Playwright screenshots for some reason.
Before: https://preview.gravity-ui.com/navigation/?path=/story/components-mobilefooter--showcase
After: https://preview.gravity-ui.com/navigation/305/?path=/story/components-mobilefooter--showcase
| @@ -103,6 +116,18 @@ $block: '.#{variables.$ns}composite-bar-item'; | |||
| height: 100%; | |||
| cursor: pointer; | |||
|
|
|||
| background: none; | |||
There was a problem hiding this comment.
Can we move the duplicate code into a mixin?
There was a problem hiding this comment.
Great catch, thank you! Fixed.
| @@ -10,12 +10,13 @@ import './Logo.scss'; | |||
| const b = block('logo'); | |||
|
|
|||
| export const Logo: React.FC< | |||
| LogoProps & {compact?: boolean; buttonClassName?: string; buttonWrapperClassName?: string} | |||
There was a problem hiding this comment.
This is breaking change. Please retarget your PR to branch v3. Also message for commits with breaking changes should be starts with feat!
kmenshov
force-pushed
the
fix-aside-header-tabbable
branch
from
07901f4
to
01b091e
Compare
…fix-aside-header-tabbable
kmenshov
changed the title
I hereby agree to the terms of the CLA available at: https://yandex.ru/legal/cla/?lang=en