Skip to content

Commit 47c0140

Browse files
committed
clean up
1 parent 812e58e commit 47c0140

2 files changed

Lines changed: 20 additions & 86 deletions

File tree

packages/react/src/ActionList/ActionList.dev.stories.tsx

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,9 @@ export const GroupWithSubtleTitleOldAPI = () => {
5252
key={user.login}
5353
selected={Boolean(assignees.find(assignee => assignee.login === user.login))}
5454
aria-checked={Boolean(assignees.find(assignee => assignee.login === user.login))}
55-
onSelect={() => {toggleAssignee(user);console.log("user",user)}}
55+
onSelect={() => {
56+
toggleAssignee(user)
57+
}}
5658
>
5759
<ActionList.LeadingVisual>
5860
<Avatar src={`https://github.com/${user.login}.png`} />

packages/react/src/ActionList/Item.tsx

Lines changed: 17 additions & 85 deletions
Original file line numberDiff line numberDiff line change
@@ -28,25 +28,15 @@ export const SubItem: React.FC<ActionListSubItemProps> = ({children}) => {
2828

2929
SubItem.displayName = 'ActionList.SubItem'
3030

31-
const ButtonItemContainerNoBox = React.forwardRef<
32-
HTMLButtonElement,
33-
React.HTMLAttributes<HTMLButtonElement> & {as?: React.ElementType}
34-
>(({children, style, as: Component = 'button', ...props}, forwardedRef) => {
35-
// If a custom component is provided via 'as', use it (e.g., React Router's Link)
36-
// Otherwise, render a button
37-
if (Component !== 'button') {
31+
const ButtonItemContainerNoBox = React.forwardRef<HTMLButtonElement, React.HTMLAttributes<HTMLButtonElement>>(
32+
({children, style, ...props}, forwardedRef) => {
3833
return (
39-
<Component ref={forwardedRef} style={style} {...props}>
34+
<button type="button" ref={forwardedRef as React.Ref<HTMLButtonElement>} style={style} {...props}>
4035
{children}
41-
</Component>
36+
</button>
4237
)
43-
}
44-
return (
45-
<button type="button" ref={forwardedRef as React.Ref<HTMLButtonElement>} style={style} {...props}>
46-
{children}
47-
</button>
48-
)
49-
})
38+
},
39+
)
5040

5141
const DivItemContainerNoBox = React.forwardRef<HTMLDivElement, React.HTMLAttributes<HTMLDivElement>>(
5242
({children, ...props}, forwardedRef) => {
@@ -70,35 +60,18 @@ type LinkItemContainerProps = React.HTMLAttributes<HTMLAnchorElement> &
7060
const LinkItemContainerNoBox = React.forwardRef<HTMLAnchorElement, LinkItemContainerProps>(
7161
({children, onClick, userOnClick, inactiveText, as: Component = 'a', ...rest}, forwardedRef) => {
7262
const clickHandler = (event: React.MouseEvent<HTMLElement>) => {
73-
console.log('[LinkItemContainerNoBox] Click handler called', {
74-
hasOnClick: !!onClick,
75-
hasUserOnClick: !!userOnClick,
76-
defaultPrevented: event.defaultPrevented,
77-
Component,
78-
href: rest.href,
79-
target: event.target,
80-
currentTarget: event.currentTarget,
81-
targetTag: (event.target as HTMLElement).tagName,
82-
currentTargetTag: (event.currentTarget as HTMLElement).tagName,
83-
})
84-
85-
if (onClick) {
86-
console.log('[LinkItemContainerNoBox] Calling onClick (Item clickHandler)')
87-
onClick(event)
88-
console.log('[LinkItemContainerNoBox] After onClick', {defaultPrevented: event.defaultPrevented})
89-
}
90-
91-
if (userOnClick) {
92-
console.log('[LinkItemContainerNoBox] Calling userOnClick (NavList onClick)')
93-
userOnClick(event as React.MouseEvent<HTMLAnchorElement>)
94-
console.log('[LinkItemContainerNoBox] After userOnClick', {defaultPrevented: event.defaultPrevented})
95-
}
63+
onClick && onClick(event)
64+
userOnClick && userOnClick(event as React.MouseEvent<HTMLAnchorElement>)
9665
}
9766
if (inactiveText) {
98-
return <span {...(rest as React.HTMLAttributes<HTMLSpanElement>)}>{children}</span>
67+
return (
68+
<span ref={forwardedRef} {...(rest as React.HTMLAttributes<HTMLSpanElement>)}>
69+
{children}
70+
</span>
71+
)
9972
}
10073
return (
101-
<Link as={Component} {...rest} onClick={clickHandler} ref={forwardedRef}>
74+
<Link as={Component} ref={forwardedRef} {...rest} onClick={clickHandler}>
10275
{children}
10376
</Link>
10477
)
@@ -176,11 +149,7 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
176149

177150
/** Detect if this should be a link item based on props - needed early for role inference */
178151
const isLinkItem = Boolean(
179-
props.href ||
180-
// Only treat as link item if it's explicitly using LinkItem pattern (has to prop AND _PrivateItemWrapper)
181-
// or if role is explicitly 'link'. Don't treat as='a' alone as a link item because
182-
// NavList uses as='a' for button items with router links
183-
role === 'link',
152+
props.href || props.to || (typeof props.as === 'string' && props.as.toLowerCase() === 'a') || role === 'link',
184153
)
185154

186155
/** Infer item role based on the container */
@@ -218,18 +187,10 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
218187

219188
const clickHandler = React.useCallback(
220189
(event: React.MouseEvent<HTMLElement>) => {
221-
console.log('[Item clickHandler] Called', {
222-
disabled,
223-
inactive,
224-
loading,
225-
hasOnSelectUser: !!onSelectUser,
226-
defaultPrevented: event.defaultPrevented,
227-
})
228190
if (disabled || inactive || loading) return
229191
onSelect(event, afterSelect)
230-
console.log('[Item clickHandler] After onSelect', {defaultPrevented: event.defaultPrevented})
231192
},
232-
[onSelect, disabled, inactive, afterSelect, loading, onSelectUser],
193+
[onSelect, disabled, inactive, afterSelect, loading],
233194
)
234195
const keyPressHandler = React.useCallback(
235196
(event: React.KeyboardEvent<HTMLElement>) => {
@@ -299,27 +260,6 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
299260
// 2. isLinkItem - direct Item usage with link props (href, to, etc.)
300261
// 3. default - regular button/div behavior with ActionListBaseItemProps
301262

302-
// Debug logging
303-
console.log('[ActionList.Item Debug]', {
304-
isLinkItem,
305-
listSemantics,
306-
buttonSemantics,
307-
itemRole,
308-
listRole,
309-
hasPrivateWrapper: !!_PrivateItemWrapper,
310-
ItemWrapperType: _PrivateItemWrapper
311-
? 'PrivateWrapper'
312-
: isLinkItem
313-
? 'LinkItemContainer'
314-
: listSemantics
315-
? 'DivContainer'
316-
: 'ButtonContainer',
317-
propsKeys: Object.keys(props),
318-
hasTo: !!props.to,
319-
hasHref: !!props.href,
320-
hasOnClick: !!props.onClick,
321-
})
322-
323263
const containerProps =
324264
_PrivateItemWrapper || isLinkItem
325265
? //TODO: need to pass props excluding link related ones to container props, otherwise aria-current, as and href or style would be passed to <li> and fail the src/NavList/NavList.test.tsx
@@ -333,25 +273,17 @@ const UnwrappedItem = <As extends React.ElementType = 'li'>(
333273
? menuItemProps
334274
: isLinkItem
335275
? {
336-
...props,
337276
...menuItemProps,
338277
inactiveText,
339278
userOnClick: props.onClick as ((event: React.MouseEvent<HTMLAnchorElement>) => void) | undefined,
279+
ref: forwardedRef,
340280
}
341281
: !listSemantics && {
342282
...menuItemProps,
343283
...props,
344284
ref: forwardedRef,
345285
}
346286

347-
// Debug logging
348-
console.log('[ActionList.Item wrapperProps]', {
349-
wrapperPropsKeys: Object.keys(wrapperProps || {}),
350-
hasOnClick: !!(wrapperProps && 'onClick' in wrapperProps),
351-
listSemantics,
352-
isLinkItem,
353-
})
354-
355287
return (
356288
<ItemContext.Provider
357289
value={{

0 commit comments

Comments
 (0)