search

bleroy / Nwazet.Commerce

Idiomatic commerce module for Orchard CMS.
BSD 3-Clause "New" or "Revised" License
26 stars 21 forks source link

display for Attributes. #135

Open MatteoPiovanelli-Laser opened 7 years ago

MatteoPiovanelli-Laser commented 7 years ago

Right now, by default, attributes aren't really displayed. The ProductPartDriver "asks" for GetAttributeDisplayShape, and that returns stuff specific to those attributes. However, ProductAttributes are ContentItems. They may have a lot more to show, but none of it is displayed. I could just call the BuildDisplay for the attributes in the alternate, and that is fine.

However, I think it could make sense to review this part and see whether it can be made simpler, so it uses the more conventional Orchard way and goes through the driver.Display method for both ProductAttributesPart and ProductAttributePart. The end goal for me is to give back to attributes the flexibility of other ContentItems in terms of adding stuff to them.

bleroy commented 7 years ago

In principle, that sounds great, but I'm a little worried about potential select n+1 consequences. If we're super-careful and do some A/B perf checks, go for it though.

MatteoPiovanelli-Laser commented 7 years ago

Tbh, I am not 100% sure I would implement this yet. It's just something i thought about today while designing a site. I'll have to dig in more when I have the time. Since there is a viable workaround (doing builddisplay from an alternate), this is not a very high priority for me at the moment.