WIP Fixes menu issues & some UI for Joomla 4.0 (beta)

[vingle]

Handles Civi menu top & bottom, Joomla side menu expanded & shrunk, at three viewports. Tested on Joomla 4 Beta 5 with Civi 5.32.2 & Joomla 3.9.23 for regressions.

Overview

Joomla 4 has a new admin theme. This PR integrates CiviCRM, notably the Civi Menu, with it, at desktop, mobile and tablet widths. Also removes padding around content container.

Before

After

Technical Details

This targets Joomla 4.0 with the body class .layout-default, ie body.admin.com_civicrm.layout-default and this seems to avoid impacting Joomla 3.x, which uses body.admin.com_civicrm. If there are parts of Joomla 4 or CiviCRM on Joomla that don't use .layout-default (haven't seen one yet), then these changes wouldn't work.

Comments

The Joomla sidebar takes up a lot of room at mobile viewport, might be worth hiding at max-width: 767px (same for Joomla 3.x - which could also benefit with less padding at mobile width).

[civibot]

(Standard links)

• If this is your first pull-request for CiviCRM, please browse CONTRIBUTING.md for information about the development and testing processes.
• If you are reviewing this pull-request, you may wish to consult the test sites and the Review Standards (long template, short template).

[colemanw]

This looks great visually, but I'd like to suggest some code restructuring. A little background:

• Currently the joomla-specific menu css is located in css/menubar-joomla.css.
• The joomla.css file is rather dated and I'd rather not add to it.
• The breakpoint variable is set by default at 767(min) and 768(max) which based on your PR is still the correct point for Joomla 4.
• We have a convention of creating separate files for different CMSs (or major versions of CMSs) so e.g. there is a css/menubar-drupal7.css and css/menubar-drupal8.css file.

Based on that, I recommend:

1. Placing all this code into a new file called css/menubar-joomla4.css.
2. Rename the existing file css/menubar-joomla3.css.
3. Follow the convention in the existing file to use variables rather than hard-coded breakpoints, (but I think we can make an exception for the one you set at 575px and hard-code that one cause there's no equivalent in the other CMSs).
4. Add a tiny bit of php logic to check the Joomla version and load up the correct file here:

   civicrm-core/CRM/Core/Resources.php

   Lines 469 to 470 in 87edc8d

   	$cms = strtolower($config->userFramework);
   	$cms = $cms === 'drupal' ? 'drupal7' : $cms;

[vingle]

Thanks. Not all of this relates to menus - the first 4 selectors are other bits of J4 tidying. Can they stay in the Joomla.css file?

I didn't know there were breakpoint variables - where are the :root variable values defined out of interest?

[colemanw]

It's not true SCSS just fairly simple str_replace:

civicrm-core/CRM/Core/Resources.php

Lines 482 to 493 in 87edc8d

	$vars = [
	'$resourceBase' => rtrim($config->resourceBase, '/'),
	'$menubarHeight' => $params['height'] . 'px',
	'$breakMin' => $params['breakpoint'] . 'px',
	'$breakMax' => ($params['breakpoint'] - 1) . 'px',
	'$menubarColor' => $menubarColor,
	'$menuItemColor' => $params['menuItemColor'] ?? $menubarColor,
	'$highlightColor' => $params['highlightColor'] ?? CRM_Utils_Color::getHighlight($menubarColor),
	'$textColor' => $params['textColor'] ?? CRM_Utils_Color::getContrast($menubarColor, '#333', '#ddd'),
	];
	$vars['$highlightTextColor'] = $params['highlightTextColor'] ?? CRM_Utils_Color::getContrast($vars['$highlightColor'], '#333', '#ddd');
	$e->content = str_replace(array_keys($vars), array_values($vars), $content);

But the $params can all be overridden using hook_civicrm_buildAsset(), which is nice for extensions, or e.g. a CMS-specific override like this one for WP:

civicrm-core/CRM/Utils/System/WordPress.php

Lines 987 to 992 in 9431cbf

	public function alterAssetUrl(\Civi\Core\Event\GenericHookEvent $e) {
	// Set menubar breakpoint to match WP admin theme
	if ($e->asset == 'crm-menubar.css') {
	$e->params['breakpoint'] = 783;
	}
	}

[colemanw]

Thanks. Not all of this relates to menus - the first 4 selectors are other bits of J4 tidying. Can they stay in the Joomla.css file?

The only 2 styles I see in your diff that are not menu-related are:

body.admin.com_civicrm.layout-default #subhead {
	display: none;
}
body.admin.com_civicrm.layout-default #content > .row > .col-md-12 {
	padding: 0;
}

The rest appears to be related to the menubar or the space it displaces.

[vingle]

It's not true SCSS just fairly simple str_replace:

Very nice - it looks like this is only accessible to the menubar css files? Can think of some other areas it would be useful, like form labels.

is the cascade order of the various css files set anywhere? When theming, focusesing on civicrm.css, I often need !important to override something which is loaded later in the cascade in say Dashboard.css or searchForm.css – so I've not been a big fan of separate files.

The only 2 styles I see in your diff that are not menu-related are:

Fair point - and #subhead is really about space displacement too.

As Joomla 4 isn't at Release Candidate yet – and the admin theme is unpopular enough to maybe face bigger changes, maybe best to wait a bit with implementing this further, especially with other outstanding Joomla 4 issues.

[colemanw]

is the cascade order of the various css files set anywhere?

Yes, the more general civicrm.css should be loading before more specific styles for joomla.css or menubar.css. When adding your own styles you can set their weight to control loading order.

[aydun]

Did this reach a conclusion? Seems like an improvement even if further changes are needed later.

[colemanw]

@vingle are you able to make the suggested changes?

[vingle]

@vingle are you able to make the suggested changes?

those aren't quick changes for me to make I'm afraid.

it's on a list to do once Joomla 4 is at Release Candidate (and they might change their admin theme before in which case it would need redoing anyway).

[colemanw]

Ok marking this PR as WIP for now.

[eileenmcnaughton]

@vingle @colemanw is there any portion of this we can resolve / get merged now - or perhaps we should close this & track in gitlab?

[vingle]

I could just put most of this in the menubar-joomla.css and make a new PR?

Arguably the J3 to 4 jump isn't as big as D7 to 8 - so maybe doesn't need its own css file (like WP 4 to 5 didn't)? Also, there'd be a bunch of selectors in the v3 that will still be needed in the v4 so splitting the file makes this a bigger job to try to figure what is needed for 4 and what isn't.

[eileenmcnaughton]

@colemanw does ^^ meet minimum requirements? @vingle am I right in thinking you are less comfortable with making the php changes @colemanw suggested than the css ones?

[vingle]

@eileenmcnaughton that's true.

But more than that - perhaps because I've not seen the background to this convention, I don't understand the advantage of having menubar-joomla3 and menubar-joomla4 - which will contain both much of menubar-joomla3 and this PR. It sounds like duplication of css - and a bit of work ontop of this PR to figure what isn't needed in menubar-joomla4 from menubar-joomla3.

[eileenmcnaughton]

@vingle OK - that makes the blocker clear - I'm gonna take a stab that the goal was to have new code for joomla 4 without impacting joomla3 (I'm not sure whehter that is out of caution or because there is reason to think it would have an impact)

Assuming we want to be able to develop css for j4 without having to worry about the impact on j3 then it would not necessarily require separating out - all the j3 stuff could be duplicated into the j4 file - with code comments making the difference between deliberate j4 stuff & copy & paste so that future editors can go 'oh they didn't make an intentional decision to add this so it's easy to make a case to remove it)

Obviously @colemanw can clarify better than I can

[vingle]

Yes that makes sense, tho that's why I prefixed the new css with

body.admin.com_civicrm.layout-default…

.admin targets backend pages, .com_civicirm targets Joomla and .layout-default targets (as far as I can tell, tho I've not found the documentation) Joomla 4. So the CSS in this PR didn't impact J3 when I tested (likewise it shouldn't resolve on any of the other CMSs if it was somehow loaded with them).

[eileenmcnaughton]

OK - let's see if your last comment changes @colemanw's understanding of the best approach

[colemanw]

I think there's a trade-off. You are correct that there would be some duplication of code between the J3 & J4 css files.
OTOH, the downside is that loading them both leads to potential conflicts (where the css which is needed in one theme causes problems in another), and it also leads to obsolete css hanging around in our codebase for years with no one remembering why it's there or knowing if it's safe to remove.

[colemanw]

That said, I don't want to block these fixes from going in, so go ahead with your other PR.

[vingle]

Thanks @colemanw & @eileenmcnaughton, I've just made a new PR here: #20401 - so feel free to close this.

I ended up redoing the css from this PR - and changing the below-menu position slightly to limit future problems if the sidebar gets a new width.

I do agree with the goal of trying to clean up some of the mountain of legacy css; and did realise while doing this how having separate joomla3.css and joomla4.css would be a nice chance to start that, but it requires more time. Changes are all under a /* Joomla 4 */ comment to make it easier to separate them at some point in the future.

[vingle]

Actually.. just realised I can close this!