Conditional vertical align for submenu items

[paulkirspuu]

Follow-up to https://github.com/conversionxl/aybolit/pull/253.

So currently we're limiting the alignment decision to whether submenu is larger or not vs parent. Could we add a condition there to override the decision to do it manually via a parent menu class?

---

I'll show what I mean below.

This could very well be aligned, but its not because submenu is smaller than parent:

Creates inconsistency and doesn't fully feel like a mega-menu.

[lkraav]

This could very well be aligned, but its not because submenu is smaller than parent:

I'm unclear on what's not aligned on your screenshot?

[paulkirspuu]

Right, that's an inspector edited prototype. Wanted to bring out that if it was aligned, it would look just fine.

This is the reality:

[anoblet]

Would you set a class on the entire parent menu, or just the selected item? Where would this be defined, and do we already have logic in place to append a class to a menu or menu item?

[paulkirspuu]

This would be applied to closest parent menu item, in current case it would be "Digital marketing". I was thinking WP menus admin, would that work?

[lkraav]

I think forcing via classes is a bad idea here, creates management nightmare, and nobody will understand why some menus work or don't work correctly.

Automatic algo needs to improve here, instead.

What is specifically going wrong with this real world sample?

Maybe there needs to be a "smaller?" threshold of some kind? How can robots determine this sample should be aligned up top?

[anoblet]

@paulkirspuu

From your screenshot it looks like the secondary menu is only a few pixels shorter than the parent menu. It would be easy to add a tolerance to the logic though what would we want it to look like? Should it be static (25px) or percentage (75%)?

[paulkirspuu]

For current case the hotfix works for me, so I'm all for patching it up and deploying live right now.

Meanwhile for future cases we'll need to think of something else, because if the parent menu is last item, it would break.

[lkraav]

Maybe auto-expanding anything shorter is the way to go here. Create a try branch @anoblet?

[lkraav]

After eye-balling this some more in #265, I've reached a decision:

• this is a problem to be solved at IA level, not code. That blank space just looks all wrong and there's really no way to right it, not for us, not for Udemy. It visually blows.
• :point_up: for time being, it's up to marketing to design an IA where parent menu should never be longer than the child - this highly likely serves as an immediate general menu content sanity check, too.
• for "close calls", like described in issue opener, figure out a way to add some a some sales paragraph copy to child menu opener item, to bump its natural height past its parent.

Only thing that #265 could do for us, is to ensure wide attribute check @anoblet.

cc @heshfekry

PS @anoblet We should also keep this full "height expand" implementation branch copy around to revisit at some later point, just in case, but let's avoid adding more complexity with it for now.

[paulkirspuu]

Just so happened that MDs have been updated with additional courses since last check, effectively fixing this issue in hand.