search

conversionxl / aybolit

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

refactor(cxl-ui): cxl-marketing-nav logo placement; border color #379

Closed pawelkmpt closed 6 months ago

pawelkmpt commented 6 months ago

Add logoBar property to define on which menu bar CXL logo should be displayed. It's needed when we want to display just a single primary menu bar (white), which didn't have CXL logo displayed by default.

pawelkmpt commented 6 months ago

Task linked: CU-86ayraehj Front-end bit of nav bar facelift

github-actions[bot] commented 6 months ago

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 69.69 KB (+0.04% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.89 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 28.85 KB (+0.06% 🔺)
packages/cxl-ui/pkg/dist-web/vendor.js 140.5 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 252.08 KB (+0.02% 🔺)
pawelkmpt commented 6 months ago

Rather than creating an enum here, I would suggest a boolean such as isGlobal, or isPrimary.

Why? I'd have 2 properties to take care of then and more complex code on the PHP side.

anoblet commented 6 months ago

Rather than creating an enum here, I would suggest a boolean such as isGlobal, or isPrimary.

Why? I'd have 2 properties to take care of then and more complex code on the PHP side.

I may be misunderstanding the context, though I only see two options. If there were three or more I could understand an enum..

I could be entirely incorrect here, I'm just trying to be helpful. Two strings would have 5 outputs as opposed to 4: TT TF FF FT UF. UF being an undefined string.

pawelkmpt commented 6 months ago

I may be misunderstanding the context, though I only see two options. If there were three or more I could understand an enum..

We will show just one nav bar now: primary and we need CXL logo on this bar instead of the global.

I don't want to move logo placement permanently though, I'd like to control it via property. That's why string property with the name as in such case I can show the logo on any bar I want and do it without modifying aybolit code, just pass param in PHP.

I could be entirely incorrect here, I'm just trying to be helpful.

Your point of view is always welcome :)

Two strings would have 5 outputs as opposed to 4: TT TF FF FT UF. UF being an undefined string.

I don't understand this part.

anoblet commented 6 months ago

KISS - There's so much to be done here.

There's a maintenance burden here.