-
Notifications
You must be signed in to change notification settings - Fork 292
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 tabs list bug - visible focus displayed when clicking out side and then clicking on one tab #537
Conversation
…to storybook/hadas/icon-button-dropdown � Conflicts: � src/components/Dropdown/__stories__/dropdown.stories.mdx
…to storybook/hadas/icon-button-dropdown
…to bug/hadas/tabs-focus � Conflicts: � src/components/IconButton/__stories__/IconButton.stories.mdx
@@ -57,7 +57,7 @@ | |||
} | |||
|
|||
&.tab-focus-visible-inset { | |||
@include focus-style-inset(3px, 3px); | |||
@include focus-style-css-inset(3px, 3px); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the other mixin depend on using focus-visible class name which had different purpose (we inject it when we do real focus for old browsers support)
setFocusTab(-1); | ||
} | ||
const getItemByIndex = useCallback(index => children[index], [children]); | ||
const disabledIndexes = useMemo(() => Array.from(disabledTabIds), [disabledTabIds]); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm probably missing something, but shouldn't disabledTabIds
just be called disabledTabIndexes
? what's the difference here between an id and an index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the ids are index but saving in a set object which not support by the hook.
|
||
const keyboardContext = useContext(GridKeyboardNavigationContext); | ||
|
||
const onArrowNavigation = direction => { | ||
setIsUsingKeyboardNav(true); | ||
isUsingKeyboardNav.current = true; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
at first I change it to ref because I wanted to add some events and I was afraid from changing react state and renders side effects, but then I saw that there no need to add events.
I keep it as ref because it save some not needed renders but I can undo this change
…to bug/hadas/tabs-focus � Conflicts: � src/components/Tabs/TabList/TabList.jsx
No description provided.