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] navigation height #265

Open anoblet opened 1 year ago

anoblet commented 1 year ago

https://github.com/conversionxl/aybolit/issues/264

github-actions[bot] commented 1 year ago

size-limit report 📦

Path Size
packages/cxl-ui/pkg/dist-web/cxl-ui.js 33.46 KB (+0.07% 🔺)
packages/cxl-ui/pkg/dist-web/cxl-ui-jwplayer.js 11.87 KB (0%)
packages/cxl-ui/pkg/dist-web/cxl-ui-playbooks.js 24.9 KB (0%)
packages/cxl-ui/pkg/dist-web/vendor.js 125.6 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 196.99 KB (+0.02% 🔺)
lkraav commented 1 year ago

Code is great, but I can't help to think if it somehow be visually bettered :thinking: what are we missing about mega-menu layout basics - if anything?

image

Weird part is the obvious empty space we create at focus (parent menu item) area.

heshfekry commented 1 year ago

Code is great, but I can't help to think if it somehow be visually bettered 🤔 what are we missing about mega-menu layout basics - if anything?

Udemy has a rather clean menu similar to this.


image

- Our shadow work is a bit over the top. Needs to be toned down in both size and darkness. A smaller lighter shadow.

image

- We need a little separation or at least appear under the main navbar, not overlap

image

- They align text to the left and the arrows to the right eating up some of the margin on that left hand side.

image

lkraav commented 1 year ago

This PR should focus only on alignment, and associated white-space.

Udemy example shows some people just live with massive negative space below menu items.

Let's also note that Google apps avoid mega-menus image

As an SOP, we should probably research well known expert centers like NNGroup and Baymard first. So I'll be looking through https://www.nngroup.com/articles/mega-menus-work-well/ for one, and will post findings later.

lkraav commented 1 year ago

One simple action item here could be to improve our limited menu item sample to be more realistic.

These Item #1 titles are too lazy of an example, because

heshfekry commented 1 year ago

sample to be more realistic

Indeed our reality makes the below almost entirely redundant.

massive negative space below menu items.

heshfekry commented 1 year ago

I am wondering if aligning the Child menus to top already solves the main issues though. no need to constrain the height of the menu if it introduces unnecessary complexity with scrolling and etc?

image

lkraav commented 1 year ago

no need to constrain the height of the menu if it introduces unnecessary complexity with scrolling and etc?

We are not. Our only problematic case is when sub-menu is (significantly) shorter than parent.

But do we even have that live, ever?

And perhaps it signals a problem in itself, that should be fixed at IA level.

anoblet commented 1 year ago

I added additional navigation items as well as used realistic labels. Moving to the REST API should be a priority for us.

Most mega menus I've encountered have a fixed height(meaning each menu and child menus have the same height). We need to be careful though and add checks so that our logic only runs on wide screens since we haven't tested mobile and the more we customize this, the more likely we will run into issues with responsiveness.

lkraav commented 1 year ago

Most mega menus I've encountered have a fixed height(meaning each menu and child menus have the same height). We need to be careful though and add checks so that our logic only runs on wide screens since we haven't tested mobile and the more we customize this, the more likely we will run into issues with responsiveness.

Correct, we trigger Vaadin overrides only on wide.

I'm still not convinced that mobile Back button is worth the complicated code it brought in.

heshfekry commented 1 year ago

I'm still not convinced that mobile Back button is worth the complicated code it brought in.

@lkraav If i remember correctly this was a rogue project by syed added unnecessecarily into the spec of sticky nav bar task.

https://app.clickup.com/t/3rgnekt