NcBreadcrumb does not render <a> anymore when to is provided

[raimund-schluessler]

In #5077 we messed up a bit and introduced a regression while trying to fix the breadcrumbs behaviour when neither to nor href are provided.

The issue is that before, the breadcrumbs would always render an <a>, both for to and href, because that is what <router-link> does by default. So also when to is provided, we would have an <a> with the correct href set in the DOM. This is what e.g. the files app also relies on, we want the breadcrumbs to be <a>s there, also if it is really a <router-link>.

Now, NcButton behaves different. It by default renders a <button>, just when an href is provided (and no to) it renders an <a>. This breaks the breadcrumbs behaviour, as the crumbs are not <a>s anymore, but <button>s. Navigation still works though, but one cannot open the crumb in a new tab like a link.

The question is now how to fix this.

• One option would be to adjust NcButton to render an <a> if to is provided. But that might classify as breaking change to NcButton, although one could also consider it a fix. I am not sure here.
• The more conservative, but also more complex option would be to introduce a prop to configure NcButton to render an <a> or a <button> when to is provided.

@ShGKme @susnux @emoral435 For input. We will need to fix this before the next patch or minor release to not mess up the files app.

[emoral435]

AH, I see what your are suggesting and referencing. I can propose a third, different change:

We can revert the change made from that PR. We keep the same functionality as before if we have a 'to' or 'href' prop passed in, but if we do not have either (v-else), we can then render the NcButton. This makes it so that we do not have to change the NcButton component.

I am open to all these changes, though, as making NcButton more flexible is also very nice 👍with the other proposed solutions, we would just have to make sure that everywhere that NcBreadcrumbs gets used has the correct new props to logically conclude what element needs to be rendered.

Thoughts?

[raimund-schluessler]

Yes, reverting it completely and only using NcButton if neither to nor hrefare present would also be an option. But since NcButton internally anyway decides depending on to and href, I tend to keep it as it is. Altough I don't have a strong preference here, to be honest.

we would just have to make sure that everywhere that NcBreadcrumbs gets used has the correct new props to logically conclude what element needs to be rendered.

The interface of NcBreadcrumb should not change, no matter which solution we decide for. Otherwise, it would be a breaking change (without the need for it even).

[ShGKme]

Oh, I really missed that we render <button> for links when they are router links...

• One option would be to adjust NcButton to render an <a> if to is provided. But that might classify as breaking change to NcButton, although one could also consider it a fix. I am not sure here.

According to the documentation:

Providing the to attribute turns the button component into a router-link element. Takes precedence over the href attribute.

But what is not said and what is unexpected to me — that this <router-link> renders a <button> (while the default behavior for <router-link> is to render a link).

I'd consider it a bug. Even if a link uses History API, it still has a link semantics, should have a context menu, browser support and ect.

@skjnldsv wasn't it done intentionally?

[raimund-schluessler]

I will prepare a PR to let NcButton render a link / <a> instead of <button> when the to prop is provided.

[raimund-schluessler]

See #5091.

[skjnldsv]

@skjnldsv wasn't it done intentionally?

Don't think so no. Not being able to open in a new tab is a pretty good regression.
When in doubt, stay as close as official html rfc and specifications, if the ressource has its own url, then it should be an a 🤷

[raimund-schluessler]

The main question is whether an NcButton that acts as a router-link should also render an a.

[skjnldsv]

The main question is whether an NcButton that acts as a router-link should also render an a.

I think it should imho