Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

allow easier mega-menu like items #9159

Closed
wants to merge 1 commit into from
Closed

allow easier mega-menu like items #9159

wants to merge 1 commit into from

Conversation

pine3ree
Copy link
Contributor

@pine3ree pine3ree commented Sep 6, 2016

I often use .menu.[size]-[num]-up li.column to build mega menus. This small change will allow more complex mega menus for dropdown list items instead of equally divided boxes only, without redefining all grid column classes ad hoc.

@kball
Copy link
Contributor

kball commented Sep 30, 2016

This makes sense to me. @andycochran @brettsmason @JeremyEnglert as our SCSS experts, any objections?

@@ -207,7 +207,7 @@ $dropdownmenu-border-width: nth($dropdownmenu-border, 1);
}
}

> li {
> li:not('.column') {
width: 100%;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Why do we automatically apply this property on all <li> in the first place, instead of using a class ?
  • Why do we declare again every menu-related properties inside a dropdown-menu component, instead of simplifying dropdown and menu ?

Also, like here, we can't assume the behavior the user expect by the classes he uses. Filtering .column fix this particular case and nothing more, can make weird behavior, and lead to an unmaintainable spaghetti code.

@pine3ree
Copy link
Contributor Author

pine3ree commented Oct 9, 2016

@ncoden
Hello,
I am fine with adding an extra class for this but, since i am not adding an feature, just preventing the reset of the width to 100%, we would need to use it in the same way with a not selector, otherwise the menu selector wins.

I used the .column class because when i use it for building mega menus it's its own properties being reset, but i recognize that someone else could need it for other purposes.

kind regards

@andycochran
Copy link
Contributor

Yeah, this won't work for semantic classes. 😞

@kball
Copy link
Contributor

kball commented Oct 31, 2016

Based on @ncoden and @andycochran 's feedback, I think this is probably not the right approach and we should close this. It sounds like the right approach is to rework the approach to dropdown menu to not make assumptions based on selector.

@kball kball closed this Oct 31, 2016
@pine3ree
Copy link
Contributor Author

@kball
I agree...it was a shurtcut I use to avoid adding kbytes of css for this simple goal. The approach you are suggesting is very welcome.
kind regards

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants