Skip to content
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

Link: do not render <p> #1924

Closed
1 task
theetrain opened this issue Mar 6, 2024 · 2 comments · Fixed by #1926
Closed
1 task

Link: do not render <p> #1924

theetrain opened this issue Mar 6, 2024 · 2 comments · Fixed by #1926
Labels
breaking changes bug Something isn't working Svelte 5

Comments

@theetrain
Copy link
Collaborator

theetrain commented Mar 6, 2024

Original report: #1908 (comment)

Issue

Svelte 5 yields a compiler error for <Link> because <div> is an invalid descendent of <p>, which is correct as per W3 spec.

I recommend we align behaviour with Carbon React:

  • When disabled, render an <a aria-disabled="true"> (⚠️ breaking change since <p> has different layout behaviour)

Reference:

{#if disabled}
<!-- svelte-ignore a11y-no-noninteractive-element-interactions -->
<p
bind:this="{ref}"
class:bx--link="{true}"
class:bx--link--disabled="{disabled}"
class:bx--link--inline="{inline}"
class:bx--link--visited="{visited}"
{...$$restProps}
on:click
on:mouseover
on:mouseenter
on:mouseleave
>
<slot />
{#if !inline && ($$slots.icon || icon)}
<div class:bx--link__icon="{true}">
<slot name="icon">
<svelte:component this="{icon}" />
</slot>
</div>
{/if}
</p>
{:else}

Reproduction

Demo: https://stackblitz.com/edit/sveltejs-kit-template-default-mxnwjf?file=src%2Froutes%2F%2Bpage.svelte

Screenshot:

image
@theetrain theetrain added bug Something isn't working breaking changes labels Mar 6, 2024
@metonym
Copy link
Collaborator

metonym commented Mar 6, 2024

Yes, I think it makes sense to fix this for <v1 if it's blocking Svelte 5 usage.

metonym added a commit that referenced this issue Mar 7, 2024
Closes #1924

Svelte 5 will not compile if `div` is nested inside `p` element. Refactor Link to render an `a` instead of `p` when disabled.
@theetrain
Copy link
Collaborator Author

Fix was released in v0.83.0.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking changes bug Something isn't working Svelte 5
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants