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

feat: update uikit, improve a11y for sharepopover #111

Merged
merged 9 commits into from
Nov 2, 2023

Conversation

Kyzyl-ool
Copy link
Contributor

No description provided.

@gravity-ui-bot
Copy link
Contributor

Preview is ready.

@@ -11,11 +12,14 @@ $block: '.#{variables.$ns}share-popover';
}

&__container {
@include mixins.button-reset();
@include mixins.focusable();
Copy link
Contributor

Choose a reason for hiding this comment

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

  1. Why we need this styles (button-reset and focusable) here?
  2. I think this styles broke UI:
    share-popover-bug

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  1. Because container element is button tag now. Need to reset button styles therefore.
  2. Fixed by disabling portal

import {NAMESPACE} from './cn';

let id = 1;
export function getUniqId() {
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we should rework this function and use it here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

https://github.com/gravity-ui/uikit/pull/1070 – the same function will be exported here from uikit

Copy link
Contributor Author

Choose a reason for hiding this comment

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

UPD: decided to use tooltipId from props

@Kyzyl-ool Kyzyl-ool force-pushed the feat/sharepopover-a11y branch from 278e120 to fc690ad Compare November 1, 2023 15:29
kkmch added 6 commits November 1, 2023 16:33
@Kyzyl-ool Kyzyl-ool merged commit eff2702 into main Nov 2, 2023
3 checks passed
@Kyzyl-ool Kyzyl-ool deleted the feat/sharepopover-a11y branch November 2, 2023 15:42
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.

None yet

3 participants