-
-
Notifications
You must be signed in to change notification settings - Fork 47
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
[tabs] Modernize implementation #751
Conversation
Netlify deploy preview |
4dbf848
to
16c8fbb
Compare
16c8fbb
to
683f8a9
Compare
c31706a
to
5a7f24f
Compare
This comment was marked as outdated.
This comment was marked as outdated.
5a7f24f
to
46d11b3
Compare
@@ -170,7 +172,9 @@ export function useCompositeRoot(params: UseCompositeRootParameters) { | |||
} | |||
|
|||
if (nextIndex !== activeIndex && !isIndexOutOfBounds(elementsRef, nextIndex)) { | |||
event.stopPropagation(); |
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.
@atomiks Why is stopPropagation()
needed here, and is it ever ok to force propagate it? This breaks a test
Given this structure:
<Tabs.Root onKeyDown={handleKeyDown}>
<Tabs.List>
<Tabs.Tab value={0} />
<Tabs.Tab value={1} />
</Tabs.List>
{/* tab panels */}
</Tabs.Root>
Tabs.List
is a CompositeRoot
, so on arrow keys, the event doesn't propagate and handleKeyDown
doesn't get called
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 not certain on that. It may not be necessary in most cases
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.
OK ~ let's let it propagate by default, but I've put a param around it just in case:
if (stopEventPropagation) {
event.stopPropagation();
}
46d11b3
to
17e3dcc
Compare
17e3dcc
to
bc1840f
Compare
useCompound
with Composite
55e7dd5
to
353f95a
Compare
353f95a
to
7bd5663
Compare
a2a0bfe
to
f00fd3d
Compare
f00fd3d
to
430bfb9
Compare
26bba8c
to
65b1053
Compare
65b1053
to
ab269fd
Compare
Summary
A custom metadata object can be passed into
Composite
components and hooksReplace
useCompound
withComposite
Renamenot yet, need to discuss the overall approach further [core] Standardize text direction (RTL/LTR) handling #831direction
prop todir
I have followed (at least) the PR section of the contributing guide.