search

backdrop / backdrop-issues

Issue tracker for Backdrop core.
144 stars 40 forks source link

Add a menu-styles API into core #1457

Closed klonos closed 6 years ago

klonos commented 8 years ago

After a recent comment from our friendly Monkey, I started reading a few articles and blog posts that were returned from google search for "I hate Drupal". I wanted to get a better idea of what it is that people hate about Drupal. What makes them give up and choose another CMS? Most of the things that pissed users off were things that they thought should be fairly simple tasks or offered out of the box by a modern CMS. In reality, these features in order to be implemented in Drupal 6/7 required a multitude of contrib modules. Then came a lot of reading and then a lot of fiddling and pogo-sticking around the UI and perhaps having to battle with bugs and feature requests lingering for ages in the respective issue queues (that could last for whole days or even weeks). Finally, when/if they somehow managed to make all these contrib modules and 3rd party libraries "sing" together, they had something that they thought was "plain ugly" or made their site slow.

Some of these commonly-requested features that seem to be coming up over and over in these articles over the net are a WYSIWYG editor (we have this covered now), a slideshow (will cover it in #1152), better media handling in general as well as being able to create drop-down menus, which is what this issue here is about...

Lets please pick a library or a contrib module among the many that exist for D7 and have drop-down menus be an out-of-the-box feature for Backdrop that is a breeze to set up. It doesn't have to be anything too complicated like say Megamenu.

The most installed modules that come up after a search for "menu" in d.org are:

  1. Superfish: more than 140k installations
  2. Nice Menus: more than 86k installations
  3. Responsive Menus: about 35k installations

It's interesting that the one module in the list that claims to be responsive is the one with the less installations out of the three - with a considerable difference too. So, when you add the term "responsive" to the same search, you get:

  1. Responsive Menus: about 35k installations.
  2. Responsive Dropdown Menus only about 2900 installations.
  3. Mobile Navigation: only about 2400 installations.
  4. Responsive Navigation: close to 500 installations.

Curious as to what are others using.

Also, could we perhaps adapt/reuse what we have for the admin bar?

klonos commented 8 years ago

Yeah, we won't have to code a solution from scratch ourselves + since its code is on GitHub, we can file PRs for things that we discover and fix or any features we might add.

wesruv commented 8 years ago

I'm not sure this is necessarily a one size fits all. Do we need settings for the following?

jenlampton commented 8 years ago
  • Text label for menu

We should default to Menu and let people override in the theme.

  • Icon

We should default to hamburger and let people override in the theme.

  • Ability to set a viewport width that the responsive menu should activate

Responsiveness should activate as soon as there is not enough room for the menu items - just like Admin bar and Tabs do today. There's no need for a setting in core. (We can leave that to contrib)

Is the most up-to-date work going on in https://github.com/wesruv/backdrop/tree/1457-responsive-menu-docwilmot ?

jenlampton commented 8 years ago

The scope has definitely crept a bit from "make menus responsive and drop-down capable" to "Add a menu styles API to core". I've created a new issue with the original intent at https://github.com/backdrop/backdrop-issues/issues/2107. That one is marked as a bug fix because we currently have no responsive menus in core.

I guess I'm okay with a menu styles API as long as we're talking about super simple API that's nearly invisible to the user. I don't want a first-time Backdrop user needing to know what "SlimMenu" means, for example.

The Primary navigation menu should be responsive (by default) and drop-down (by default) and when any menu block is placed anywhere it should also have options to do the same. If these options are API-like so contrib can provide alternatives - all the better. :)

I tried to take a look at the code both in @wesruv's repo but it looks like that was mixed in with the code in the menu-block PR which has already been merged into core. I have some other comments on the code, but I'd like to get a PR going so we can put code-related comments on the PR, and move them out of the "what are we doing" thread and into the "how are we doing it" one.

I am a bit worried we don't have time something to get this extensive into core. Making the current menu blocks drop-down and mobile-friendly seems a lot more manageable, but I'll dig into the code and see how far we've come.

wesruv commented 8 years ago

@jenlampton it's best practice to put the word 'menu' next to a hamburger, but in some cases there might be something more descriptive. When the menu is secondary or tertiary navigation, don't know if this is also a localization thing?

You're right on the other stuff, I've overthought this.

jenlampton commented 8 years ago

@wesruv the good thing about an API is that contrib can add any number of options like the things you've thought out (and more!) ...but not all of them need to be in core.

I still think we should default to using the word 'Menu' (and have it translated as such) but sites can theme-override or string-override that word as needed, even without a module that provides a menu style plugin. (which could provide a UI, too)

wesruv commented 8 years ago

Sounds good!

docwilmot commented 8 years ago

If we dont use an API, is the dropdown menu going to be provided by the theme? the menu block? We need some way to allow contrib to override the core dropdown code.

quicksketch commented 8 years ago

I'm going to take another look at this PR. The alternative approach at https://github.com/backdrop/backdrop-issues/issues/2107 doesn't really have anything going on in it. Honestly the "Menu styles API" is the easy part of this in my estimation. The tying into the theme and front-end work is the primary challenge.

I'm going to take another look at this and see how effective it would be merged in (with a little work).

It looks like most of Wes's changes have been merged into core as separate issues (the generic responsive resize handler, font loader, etc).

I think a better approach is to reference a library specified by hook_library_info(). This way the CSS and JS are tied together, and it's alterable in case someone wants to sub in some of their own CSS/JS.

Definitely agreed here, this is what libraries were made for.

I'm trying to take out the slimmenu implementation and sub in something that works with our system in mind.

We don't have much time to delay here. I'm not very picky about the library, if we were to treat this as a non-reusable library, we could actually replace the underlying library at a later time. If the library turns out to be great, we could make it generally available later.

There seems to be a bug right now where menu settings aren't being saved.

I'll take a look into this if it looks like this approach is solid.

If we dont use an API, is the dropdown menu going to be provided by the theme? the menu block? We need some way to allow contrib to override the core dropdown code.

Sans-API, everything would hinge off of the single checkbox suggested in [#2107], which I would think means that themes and modules would only be able to swap out the underlying library, probably through a theme function or something like hook_library_info_alter(). I'd prefer to see this API-driven approach, as I don't think the API is really the hold-up here, it's the display and integration of the menu on the front-end.

klonos commented 8 years ago

We don't have much time to delay here. I'm not very picky about the library, if we were to treat this as a non-reusable library, we could actually replace the underlying library at a later time. If the library turns out to be great, we could make it generally available later.

👍

jenlampton commented 8 years ago

@quicksketch is working on a new approach for https://github.com/backdrop/backdrop-issues/issues/2107 that is making this menu-styles API look really appealing (and should be relatively easy to drop-in).

The new Menu Styles feature (that other issue) is likely to make 1.5, but the Menu Styles API for contrib to access it (this issue) is not. That said, I'm starting to really want it! I'm going to move this issue from 1.5 to 1.6. now that we are only 3 days away from release.

klonos commented 8 years ago

I started reading this issue from top to bottom (it goes back 9 months) and I like the discussion and what is going to come out of it, but all I care for now is that we have an easy, user-friendly way of implementing drop-down menus in core, without needing to install a myriad of contrib modules and 3rd party libraries. This works well with core Bartik (and will hopefully also work well with Basis too). This was why I originally started this issue about and the fact that it is now a reality is a reason to 🎉 ...thank you all for taking part in making this happen.

jenlampton commented 8 years ago

but all I care for now is that we have an easy, user-friendly way of implementing drop-down menus in core

I wonder about this too... The reason we needed a WYSIWYG API in Drupal was because there was not a good way to do rich-text editing in core. In Backdrop, we decided to choose one library and integrate deeply, rather than providing an API so everyone could choose something different.

Now that we have a decent responsive drop-down menu library in core, are people still going to want to choose another? Is it worthwhile to provide an API right away (1.6) or should we wait and see if there is even still a demand?

wesruv commented 8 years ago

Is a dev able to modify this list: image

I think that would be acceptable, a way to add your own strategies to the menu block.

jenlampton commented 8 years ago

Right now all devs have is the usual form_alter so they would need to do some work to get the altered-in value to match with a library that's loaded (or not) and have the other fields on this page appear/disappear as they are relevant. Alternatively, the API would provide them a special hook/class that would make that whole process much easier.

wesruv commented 8 years ago

If it's not too much effort, I think it'd be great, if it's a ton of effort, I don't think it's that important.

Don't know how others might feel?

jenlampton commented 7 years ago

This hasn't gotten any attention and we are 15 days from release. However since the new responsive menu has only been in for one minor release I don't want to push this all the way to 1.x, let's slate it for 1.7 and see if anyone wants it over the next cycle.

docwilmot commented 7 years ago

Can we clarify this or close it and open a more specific issue? Is it an API to allow multiple JS menu libraries? To just override SmartMenu? An API to modify SmartMenu settings? Not clear.

jenlampton commented 7 years ago

@docwilmot I think a new issue might be in order, this one is getting quite long :) I'll clarify here and then we can decide what to do with the issues.

How about this: the API should be a new callback hook_menu_style_info (or something) that allows a contrib module to provide:

jenlampton commented 7 years ago

We haven't had any momentum on this since Simplemenus went into core. I'm not sure there's going to be a huge demand for an API until people decide they aren't happy with Simplemenus.

I'm going to remove the milestone since this issue has missed 2+ core releases. We can add it back when it becomes a priority.

docwilmot commented 6 years ago

This is over a year without comment so I suspect Simplemenus is satisfactory so far. Since this issue got so bloated, I'll close it, and we can open a new issue for a menu API if it arises.