ActionList.Item: Bug fix to support both inline and block description at the same time

[siddharthkp]

Thanks to ton to @joshblack for his help with the types!! ✨

Found a regression that was introduced in 6 months ago, but hasn't been reported yet :)

To fix that, added a backward compatible "test condition" to slots config, so that the component can be in two different based on props. Only ActionList.Item has this use case with inline and block description.

Example:

const [slots, childrenWithoutSlots] = useSlots(props.children, {
      leadingVisual: LeadingVisual,
      trailingVisual: TrailingVisual,
-     description: Description,
+     blockDescription: [Description, props => props.variant === 'block'],
+     inlineDescription: [Description, props => props.variant !== 'block'],
    })

Show code for preview

<ActionList.Item>
  Only I
  <ActionList.Description>inline description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  Only B
  <ActionList.Description variant="block">block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  I + B
  <ActionList.Description variant="inline">inline description</ActionList.Description>
  <ActionList.Description variant="block">block description</ActionList.Description>
</ActionList.Item>
<ActionList.Item>
  The everything bagel
  <ActionList.LeadingVisual><StarIcon/></ActionList.LeadingVisual>
  <ActionList.Description variant="inline">inline description</ActionList.Description>
  <ActionList.Description variant="block">block description</ActionList.Description>
  <ActionList.TrailingVisual><StarIcon/></ActionList.TrailingVisual>
</ActionList.Item>

Before	After

See also conversation from slack: https://github.slack.com/archives/C01L618AEP9/p1700085056205969

[github-actions]

size-limit report 📦

Path	Size
dist/browser.esm.js	104.22 KB (-0.03% 🔽)
dist/browser.umd.js	104.77 KB (-0.03% 🔽)

[siddharthkp]

The margin after item was added in Bug fix: ActionList item label weight and spacing if description exists #3490, this was the code:

marginBlockEnd: slots.description && slots.description.props.variant !== 'inline' ? '4px' : undefined,

The intention seems to be to only add margin when description variant !== 'inline', so block description?

I'm not sure if the condition is intentional or not, because the condition would also be true when props.variant is not passed, which defaults to inline (undefined !== 'inline')

I have removed the margin here, tagging @langermank to confirm that was the intention with Bug fix: ActionList item label weight and spacing if description exists #3490

[langermank]

Thanks for tagging me! I think the change you made is correct, mostly just by looking at the snapshots. 🚀

[langermank]

Hey @siddharthkp! Before I respond to the margin question, can you clarify how this is a regression? Its my understanding that its one or the other: inline or block. They shouldn't co-exist.. is there a use case I'm not aware of?

[siddharthkp]

Hey @siddharthkp! Before I respond to the margin question, can you clarify how this is a regression? Its my understanding that its one or the other: inline or block. They shouldn't co-exist.. is there a use case I'm not aware of?

When Vini did the initial inventory, we saw a few use cases for it. I remember one clearly because I see it often, but there were a couple more:

[pksjce]

Something I don't understand is why not have two different slots for block and inline description? Is it to avoid breaking changes?

[siddharthkp]

Yep, that would be a breaking change. We used to support it before the regression.

But also, this feels like an implementation detail, I don't personally like the idea of it leaking out into the API with ActionList.InlineDescription and ActionList.BlockDescription 😅