Shopify / dawn

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

Collapsible tabs being rendered even when no content is added #974

Open craigjamesdobson opened 2 years ago

craigjamesdobson commented 2 years ago

Describe the current behavior

When using the collapsible tabs on the product page with dynamic metafield data the tab will still be written out even if no data has been added to the product metafield

Describe the expected behavior

Only write out the collapsible tab if the dynamic data is present within the product

Version information (Dawn, browsers and operating systems)

Possible solution

Add an if statement to check for either type of content before writing out the tab

{%- if block.settings.content != blank or block.settings.page.content != blank -%}

gregjotau commented 2 years ago

I think this one makes sense @ludoboludo to add to a future fix, but not something that causes much trouble for us at least.

ludoboludo commented 2 years ago

Hi folks, thanks for bringing this up @craigjamesdobson. It's a good point 🤔

It would be specific when using metafields though, as we wouldn't want to hide blocks that are added in the theme editor. Again in the case of using metafields I see how it makes sense. Based on the value of the source linked then it would show or hide but we wouldn't want it to not show anything if the block is added and not source is linked. Because ideally you only add a block if you need it, if you don't, you remove it.

So we will have to explore this and see how it could work in all situations 🤔

craigjamesdobson commented 2 years ago

Hey @ludoboludo, thanks for the response.

The scenario that brought this up for my client was that they had dynamic metafields populated on some products but not all, so in the product template they had the collapsible tab but if the dynamic content was not populated for a specific product it wouldn't show the tab.

Obviously its up to you guys if you want to implement it but I would argue that if no content is present in the tab you would want it to be hidden?

ludoboludo commented 2 years ago

Yeah I think it makes a lot of sense in this specific scenario. Less so when it's just adding the block and you haven't added any text yet in rte or if the page you linked via the setting.

So I think what you're describing should only happen when a metafield/dynamic source is linked.

Ill run it by the team and see what people think 👍