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

Drop custom CSS for Joomla CiviCRM menu so it works with shoreditch theme #12632

Merged
merged 1 commit into from
Aug 12, 2018

Conversation

mattwire
Copy link
Contributor

@mattwire mattwire commented Aug 7, 2018

Overview

The Joomla CiviCRM menu has some CSS customisation that dates back to CiviCRM 3.2 and I think is unnecessary. It also breaks the menu when using the Shoreditch theme.

See civicrm/org.civicrm.shoreditch#147

Before

Non shoreditch theme:
bec-cave org uk_administrator__option com_civicrm task civicrm_admin reset 1

Shoreditch theme:
bec-cave org uk_administrator__option com_civicrm task civicrm_contribute reset 1

After

Non shoreditch theme:
bec-cave org uk_administrator__option com_civicrm task civicrm_admin reset 1 1

Shoreditch theme:
bec-cave org uk_administrator__option com_civicrm task civicrm_contribute reset 1 1

Technical Details

If the PR introduces noteworthy technical changes, please describe them here. Provide code snippets if necessary

Comments

Anything else you would like the reviewer to note

@civibot
Copy link

civibot bot commented Aug 7, 2018

(Standard links)

@mattwire
Copy link
Contributor Author

mattwire commented Aug 7, 2018

@lcdservices Any chance you could test this on your Joomla sites?

@lcdservices
Copy link
Contributor

@mattwire Tested and confirmed. The shoreditch menu now appears properly in Joomla.

@colemanw
Copy link
Member

@colemanw colemanw merged commit 70e04dd into civicrm:master Aug 12, 2018
@mattwire mattwire deleted the joomla_civimenu branch September 25, 2018 10:58
@vingle
Copy link
Contributor

vingle commented Oct 13, 2018

This seems to break non-Shoreditch Joomla menus (see ref: [civicrm.stackexchange...])(https://civicrm.stackexchange.com/questions/26858/anyone-else-seeing-an-overlapping-admin-civi-menu-bug-with-joomla-3-8-13-and-civ/26868#26868). I'm guessing LXIM means Shoreditch-specific changes to core should probably not break sites not running it? I will try and be more active in testing Joomla-specific changes to core (especially if you ping me).

@mattwire
Copy link
Contributor Author

@vingle Always looking for more Joomla testers :-) I did test this one with and without (as shoreditch is only an extension after all...) but looks like there may still be an issue. If you are able to identify the right fix I'll review and make sure we can get it merged.
By the way for testing, it's useful to do a test with:

  1. Normal CiviCRM menu.
  2. Accessible CiviCRM menu: https://github.com/aydun/uk.squiffle.kam
  3. Shoreditch theme enabled.

@vingle
Copy link
Contributor

vingle commented Oct 13, 2018

Thanks @mattwire. This fix https://civicrm.stackexchange.com/a/26868/3815 worked for me, but maybe breaks those extensions. I will try and test it with the Squiffle Menu (which I've never tried on Joomla and tbh wasn't aware it was compatible) and Shoreditch (which it's great to hear is now more compatible) as soon as I am able.

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

Successfully merging this pull request may close these issues.

6 participants