search

vuestorefront / storefront-ui

A frontend library for Vue and React that helps developers quickly build fast, accessible, and beautiful storefronts. Made with 💚 by Vue Storefront team and contributors.
https://storefrontui.io
MIT License
2.33k stars 455 forks source link

SfProductCard is not clickable with link provided #430

Closed mkucmus closed 4 years ago

mkucmus commented 4 years ago

on category's product listing when the link property is provided the product tile is not clickable at all. only clickable part of this is an "add to wishlist hearted link" on the top.

filrak commented 4 years ago

@leomp12 apart from the fact it's not working what was the intuition behind adding this prop? Shouldn't it be handled just by wrapping component in router-link ?

Also I'm prty sure this is incorrect:

  <component :is="link ? 'a' : 'div'" :href="link">

It should be:

  <component :is="link ? 'router-link' : 'div'" :to="link">
leomp12 commented 4 years ago

@mkucmus I'll check it, I think it was working but probably I've tested only without wishlist icon, maybe we have a bug with both together...

@filrak wraping it you would overlap wishlist icon click event, and price would also be linked, I think it's not good. With the prop we're linking only image and title. And I don't think it should render a router-link by default, it's context specific, by doing that we're setting sfui dependent of vue-router unnecessarily..

filrak commented 4 years ago

@leomp12 default config should work for most of the users and router-link is what most of the users will use. I agree that it shouldn't be hardcoded though so as we discussed on Discord lets have linkTag property defaulting to router-link

leomp12 commented 4 years ago

Another API option, I'll also accept Object on link, maybe we can just use router-link if link is an object (such as { name: 'cart' } or { path: '/cart' }), wdyt? Then we don't need linkTag prop, renders <a> when link is string and router-link when it's an object.

I usually use vue router like that, but honestly I don't know if it's the most common too..

filrak commented 4 years ago

i think it's easier to get it with explicit props. Regarding most common approach I often use Vue Router with just strings but same as you i'm not sure if it's most common way. If we don't know that we should aim for the simplest solution which is probably to have bare props.

@mkucmus @patzick wdyt? :)

leomp12 commented 4 years ago

tl;dr: check if Vue Router is present to render router-link by default :stuck_out_tongue:

On PR I'm adding linkTag prop, but I'm also checking for this.$router, if not present (Vue Router not used on instance) and this.link is a string I'm rendering <a>, otherwise I'm rendering <router-link> by default.