fix(navlist.item): Add disabled attribute to disabled NavList.Item component

[colomolo]

Purpose of PR

Adding actual disabled attribute to HTML element of NavList.Item when isDisabled is passed.

PR Checklist

• I have read the relevant readme.md file(s)
• All commits follow our Git commit message convention
• Tests are added/updated/not required
• Tests are passing
• Storybook stories are added/updated/not required
• Usage notes are added/updated/not required
• Has been tested based on Contentful's browser support
• Doesn't contain any sensitive information

[changeset-bot]

🦋 Changeset detected

Latest commit: f6aec93

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 32 packages

Name	Type
@contentful/f36-components	Patch
@contentful/f36-accordion	Patch
@contentful/f36-asset	Patch
@contentful/f36-autocomplete	Patch
@contentful/f36-badge	Patch
@contentful/f36-button	Patch
@contentful/f36-card	Patch
@contentful/f36-collapse	Patch
@contentful/f36-copybutton	Patch
@contentful/f36-core	Patch
@contentful/f36-datetime	Patch
@contentful/f36-datepicker	Patch
@contentful/f36-drag-handle	Patch
@contentful/f36-entity-list	Patch
@contentful/f36-empty-state	Patch
@contentful/f36-forms	Patch
@contentful/f36-icon	Patch
@contentful/f36-list	Patch
@contentful/f36-menu	Patch
@contentful/f36-modal	Patch
@contentful/f36-note	Patch
@contentful/f36-notification	Patch
@contentful/f36-pagination	Patch
@contentful/f36-pill	Patch
@contentful/f36-popover	Patch
@contentful/f36-skeleton	Patch
@contentful/f36-spinner	Patch
@contentful/f36-table	Patch
@contentful/f36-tabs	Patch
@contentful/f36-text-link	Patch
@contentful/f36-tooltip	Patch
@contentful/f36-typography	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
forma-36	✅ Ready (Inspect)	Visit Preview	Apr 29, 2024 3:53pm

[github-actions]

size-limit report 📦

Path	Size	Loading time (3g)	Running time (snapdragon)	Total time
CommonJS	132.89 KB (0%)	2.7 s (0%)	607 ms (-19.43% 🔽)	3.3 s
Module	129.59 KB (0%)	2.6 s (0%)	595 ms (-3.96% 🔽)	3.2 s

[denkristoffer]

We need to do some validation before passing this prop, disabled is not supported by <a>, which is the default for NavList. Are you intending to use this for button elements?

[colomolo]

We need to do some validation before passing this prop, disabled is not supported by <a>, which is the default for NavList. Are you intending to use this for button elements?

Yeah, somehow I missed that. Will check.

[colomolo]

We need to do some validation before passing this prop, disabled is not supported by <a>, which is the default for NavList. Are you intending to use this for button elements?

However:
https://github.com/contentful/forma-36/blob/main/packages/components/button/src/Button/Button.tsx#L124

I think it should be fine with disabled attribute. Wdyt?

[denkristoffer]

We need to do some validation before passing this prop, disabled is not supported by <a>, which is the default for NavList. Are you intending to use this for button elements?

However: main/packages/components/button/src/Button/Button.tsx#L124

I think it should be fine with disabled attribute. Wdyt?

@colomolo I don't think I follow, could you explain what you read from that?

The way I see it, the default tag for the NavListItem is <a>, which doesn't accept disabled. If you pass disabled here it will complain for anyone not rendering the <MenuListItem as="button" />.

[colomolo]

@colomolo I don't think I follow, could you explain what you read from that?

I mean that in Button component disabled is explicitly assigned to a element:

<a {...otherProps} {...commonProps} disabled={isDisabled}>

I just tried it and can't see any errors:

[denkristoffer]

@colomolo I don't think I follow, could you explain what you read from that?

I mean that in Button component disabled is explicitly assigned to a element:

<a {...otherProps} {...commonProps} disabled={isDisabled}>

Right, I see. So it's not going to throw a warning but just do nothing instead. In that case I think we can land this as it is for now, although ideal case would still be handling per element type.