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

CRM-21095: Make status messages work with Backdrop core CSS #41

Merged
merged 2 commits into from
Aug 23, 2017
Merged

CRM-21095: Make status messages work with Backdrop core CSS #41

merged 2 commits into from
Aug 23, 2017

Conversation

laryn
Copy link
Contributor

@laryn laryn commented Aug 22, 2017

@laryn
Copy link
Contributor Author

laryn commented Aug 22, 2017

It was also affecting the System Status notification in the footer on CiviCRM backend pages, so I've added another selector to better pinpoint the ones that need adjustment.

screen shot 2017-08-22 at 5 38 06 pm

@colemanw colemanw merged commit 8c5405b into civicrm:1.x-master Aug 23, 2017
@colemanw
Copy link
Member

Looks good to me.

@laryn laryn deleted the BackdropCSS branch August 23, 2017 02:26
@colemanw
Copy link
Member

colemanw commented Aug 23, 2017

Actually @laryn a more "civi-ish" way to add the css would be to add this function to the CRM_Utils_System_Backdrop class: https://github.com/civicrm/civicrm-core/blob/4.7.23/CRM/Utils/System/DrupalBase.php#L303-L310

Note that you'd be overriding the existing implementation so you'd need to decide whether that drupal.js file is still needed in backdrop or not and call the parent function if so.

@laryn
Copy link
Contributor Author

laryn commented Aug 23, 2017

@colemanw
You mean something like this?

  public function appendCoreResources(&$list) {
    $list[] = 'js/crm.drupal.js';
    $list[] = 'css/civicrm-backdrop.css';
  }

@colemanw
Copy link
Member

colemanw commented Aug 23, 2017

Yes. Although the drupal.js might not be relevant to Backdrop - it only contains a D7-specific hack and a D8 hack.

@laryn
Copy link
Contributor Author

laryn commented Aug 23, 2017

@colemanw I will make an alternative PR over at civicrm-core and post back here.

@herbdool
Copy link
Contributor

herbdool commented Aug 23, 2017

Yeah, seems like we can drop crm.drupal.js. Those selectors don't exist in Backdrop.

Though we might want a separate PR to make the Backdrop admin bar play nice with the CiviCRM admin bar. Prevent the dropdown menus from opening behind the CiviCRM menu.

@laryn
Copy link
Contributor Author

laryn commented Aug 23, 2017

@colemanw If the one linked below looks good, this one can be unmerged.

civicrm/civicrm-core#10891

@herbdool Thanks! I've left out the .js file. And yes, we may want to add something to this CSS file (once finalized) to fix that menu issue.

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

Successfully merging this pull request may close these issues.

4 participants