search

localgovdrupal / localgov_base

The base theme for LocalGov Drupal websites.
8 stars 14 forks source link

Added subsites_menu region. #542

Closed rupertj closed 3 months ago

justinepocock commented 3 months ago

I was in two minds about that myself.

markconroy commented 3 months ago

Hi,

I'd prefer the HTML and CSS for this to be in this theme rather than in the module. We can add HTML and CSS to the module as well if we want so that it works even if localgov_base is not the base theme being used.

Keep the variables here means we can place them all in the variables.css file for consistency, and it will make it easier for sub-themes to find them and override them.

dedavidson commented 3 months ago

Hi @markconroy The HTML and CSS in the three commits are examples though and don't do anything unless a menu called 'subsites' is created. They will have to be overridden/re-created for whatever the specific implementation calls its menus.

markconroy commented 3 months ago

Hi @dedavidson,

Yep, I've noticed that. I'm doing a rewrite of this PR at the moment, hoping to have something ready for it by EOD today, if not, I'll definitely have it ready over the weekend.

rupertj commented 3 months ago

The HTML and CSS in the three commits are examples though and don't do anything unless a menu called 'subsites' is created. They will have to be overridden/re-created for whatever the specific implementation calls its menus.

Subsites extras does create a menu called 'subsites' by default on install now.

markconroy commented 3 months ago

I've a pretty major re-write of this done now if anyone wants to review it.

I've no JS done yet. Let's get this reviewed and merged and I'll get the JS done then.

Also, related PR: https://github.com/localgovdrupal/localgov_scarfolk/pull/36 (it's just one line of code)

markconroy commented 3 months ago

A few small updates since Friday, and the JS added for the menu to toggle on/off on small screens.