conversionxl / aybolit

Lightweight web components library built with LitElement.
https://conversionxl.github.io/aybolit/
MIT License
7 stars 8 forks source link

feat(cxl-ui): [cxl-marketing-nav] check for empty description elements #255

Open anoblet opened 1 year ago

anoblet commented 1 year ago

https://app.clickup.com/t/861m7gmrz

github-actions[bot] commented 1 year ago

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 30.7 KB (+0.06% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.87 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 23.37 KB (0%)
packages/cxl-ui/pkg/dist-web/vendor.js 122.56 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js, packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js, packages/cxl-ui/pkg/dist-web/cxl-ui.js, packages/cxl-ui/pkg/dist-web/manifest.js, packages/cxl-ui/pkg/dist-web/unresolved.js, packages/cxl-ui/pkg/dist-web/vendor.js 189.66 KB (+0.01% 🔺)
paulkirspuu commented 1 year ago

Does https://deploy-preview-253--conversionxl-aybolit.netlify.app/?path=/story/cxl-ui-cxl-marketing-nav--cxl-marketing-nav have this latest change?

lkraav commented 1 year ago

I'm not convinced yet we should be masking this problem coming from PHP layer.

OTOH problem appearance is subtle, and nobody might notice + trimming input surroundings might make sense anyway. Should this also be done in PHP though?

I'll finish debugging PHP this week and make a decision.

anoblet commented 1 year ago

@paulkirspuu That PR doesn't have this latest code change.

paulkirspuu commented 1 year ago

I'm not convinced yet we should be masking this problem coming from PHP layer. OTOH problem appearance is subtle, and nobody might notice + trimming input surroundings might make sense anyway. Should this also be done in PHP though? I'll finish debugging PHP this week and make a decision.

So, shall we go for PHP or stick with this PR?

lkraav commented 1 year ago

So, shall we go for PHP or stick with this PR?

I'm still mid-debugging on that. But let's trim in PHP.