umbraco / Umbraco.Marketplace.Issues

Public issue tracker for Umbraco Marketplace
2 stars 0 forks source link

Use `<a>` instead of `<button>` for package cards #24

Closed abjerner closed 1 year ago

abjerner commented 1 year ago

Currently the cards for the various packages in the Marketplace are using a <button> element for triggering the package overlay to open. I assume this is due to the SPA like behavior of the Marketplace. Is there a reason for this?

I think using a <button> instead of an <a> has some UX disadvantages:

Could the cards be updated to use <a> elements instead? 😍

nathanwoulfe commented 1 year ago

hey @abjerner I'll see what I can do. From memory, there were routing issues with using <a> but might be able to get around those now. Agree it would be useful for the reasons you've mentioned.

nathanwoulfe commented 1 year ago

Looks like the routing issues were fixed in an element-agnostic way, switching to an anchor instead of button seems to work fine. Will do a bit more testing to make sure, but should have this ready to release soon.

nathanwoulfe commented 1 year ago

Hey @abjerner this change is on dev.marketplace.umbraco.com now, should be released early next week.

nathanwoulfe commented 1 year ago

Done and done - change is live now.

abjerner commented 1 year ago

Thanks 👍

abjerner commented 1 year ago

Has this change been reversed? Seems like the cards are all buttons again 🤔

nathanwoulfe commented 1 year ago

It was indeed reversed, related to issues where the card had nested links (ie had a button inside the card). It should be possible to make it work using anchors, will take another look.

nathanwoulfe commented 1 year ago

Will be fixed (again) in the next prod release 👍

abjerner commented 1 year ago

@nathanwoulfe thanks 👍

This is kinda a pet peeve of mine, as I think not having them as <a> goes against some of the fundamentals of the web, and prevents users like me to open them in a new tab or window.

We have the same problem for some of our own sites, as designers build cards with additional links like in this case. I'm not a fan of that either 🙃

nathanwoulfe commented 1 year ago

It's fun dealing with unknown content. I'd rather truncate the descriptions, but that can get messy too...

Agree these should be <a> for all the reasons you'd mentioned.

AndyButland commented 1 year ago

I'll close this as resolved and will be live on the next release (which I plan to do tomorrow morning).