Shopify / dawn

Shopify's first source available reference theme, with Online Store 2.0 features and performance built-in.
Other
2.48k stars 3.32k forks source link

Issue on product card (architecture problem) #3127

Open bakura10 opened 10 months ago

bakura10 commented 10 months ago

Hi,

I am assisting Maestrooo app team who is currently developing an app that integrates with product cards, and they are facing a huge challenge on Dawn. Dawn CSS makes it everything as hard as it is to prevent any app developer from extending the card. It might be done on purpose, I don't know :D.

Issue 1

The first major issue, is that Dawn has an after on the link that cover the full card:

Capture d’écran 2023-11-24 à 11 27 59

This also has a positive z-index, which means that app developers has to increase the z-index value (which is something I really discourage whenever possible) to go over this link.

Having a link set up like this is a very weird decision (it also prevents having any other links in the card, such as a vendor link, swatch links or whatever).

This approach should be reconsidered if you care about extensibility.

Issue 2

It seems that since recently, Dawn adds a transform: perspective(0):

Capture d’écran 2023-11-24 à 11 30 41

I have no idea why this has been added, this looks like me like an ugly workaround to a deeper architectural problem. Anyway, this innocent code actually has a major side effect: it creates a new stacking context.

In the case of this app, Maestrooo is trying to inject a button inside this div, but because of this stacking context, even if the button has a higher z-index, it will never go above the ::after link that covers the whole div, making the button impossible to reach.

Thanksl

ludoboludo commented 10 months ago

From what I remember, the :after approach for the link on the product card was a way to avoid having to many <a> links on the same product card leading to the same page. For accessibility purposes so things don't get announced multiple times per cards.

As for the perspective, it was to deal with some issue/behaviour with animations and shadows (potentially gradient as well 🤔). Enabling animation does create a new stacking context as well due to the transforms applied. So to mitigate some of the issue we had noticed, this was a way to help prevent more visual breaking.