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-20225 Add classes to body element when on basepage #111

Merged
merged 1 commit into from
Mar 8, 2017

Conversation

christianwach
Copy link
Member

As described on Jira and Stack Exchange, this implements the addition of generic classes to the body tag when on the WordPress basepage.

@christianwach
Copy link
Member Author

Should be possible to apply this PR to previous versions of CiviCRM without modification too.

@eileenmcnaughton eileenmcnaughton changed the title Add classes to body element when on basepage CRM-20225 Add classes to body element when on basepage Mar 8, 2017
@eileenmcnaughton
Copy link
Contributor

@kcristiano any chance you can review this?

@kcristiano
Copy link
Member

@christianwach I'll do some testing on here, it would help if you had some examples of what the classes should look like after applying the patch. On a quick before and after comparison the output looks identical.

@christianwach
Copy link
Member Author

@kcristiano I guess there is the assumption that a theme implements the body_class() function, but I believe that it's a requirement for the WordPress Theme Directory.

For me with any of the "Twenty*" themes, I see (for example) civicrm civicrm-event civicrm-event-ical added to the <body> tag when viewing 'http://domain.whatever/civicrm/?page=CiviCRM&q=civicrm/event/ical&reset=1&list=1&html=1'. If I visit '/civicrm/?page=CiviCRM&q=civicrm/pcp/info&reset=1&id=1', I see civicrm civicrm-pcp civicrm-pcp-info added.

Essentially what should be happening is that the q= arguments are parsed and the sequence of increasing-specific classes are added, meaning that CSS selectors can be added at whatever level of granularity is required.

@kcristiano
Copy link
Member

@christianwach I need more coffee before I test. Thank you, I was looking at the div classes not the body tag.

This works nicely and I agree it should be merged.

@eileenmcnaughton OK to merge

@eileenmcnaughton eileenmcnaughton merged commit 9a59084 into civicrm:master Mar 8, 2017
@christianwach christianwach deleted the CRM-20225 branch March 8, 2017 19:44
@christianwach
Copy link
Member Author

@eileenmcnaughton Thanks! I'll report back to the chap on SE.

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.

4 participants