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

Hook to alter menubar css variables & fix breakpoint in WP #14135

Merged
merged 4 commits into from
May 2, 2019

Conversation

colemanw
Copy link
Member

Overview

Makes the menubar more flexible and configurable; fixes a couple issues with breakpoints.

Before

  • Menubar breakpoint hardcoded at 768px which works for Drupal & Joomla but causes weird problems on WP with screen widths between 768 - 783px.

After

  • Generic hook added to allow altering of dynamic asset params
  • Several menubar variables exposed to the hook
  • Implemented hook internally to modify breakpoint to match WP admin theme

@civibot
Copy link

civibot bot commented Apr 26, 2019

(Standard links)

@civibot civibot bot added the master label Apr 26, 2019
@colemanw colemanw changed the title Menubar vars Hook to alter menubar css variables & fix breakpoint in WP Apr 26, 2019
@colemanw
Copy link
Member Author

@kcristiano would you be able to verify this works for you in WP?

With the civicrm menubar displaying underneath the WP admin bar...
Before: Shrinking your screen width to just under 783 will collapse the WP admin menu but not the Civi menu, which looks weird.
After: Civi menubar collapses below 783 to stay in sync with WP.

Tip: In Chrome if you hit F12 to open the console and then position the console on the side instead of along the bottom of the window, you can adjust your screen width by dragging the console divider line and it will conveniently show you the screen dimensions as you're doing it.

@kcristiano
Copy link
Member

@colemanw With patch applied:

783px:

image

< 783px:

image

But less than 783 px on add contact screen: no menu

image

New Mailing: No menu

image

@colemanw
Copy link
Member Author

Thanks for testing @kcristiano - I can't reproduce that issue on my local WP demo site on those screens. The fact that it works for you on some screens but not others suggests there might be a caching issue?

@kcristiano
Copy link
Member

@colemanw It may have been caching. I think it's better in than out and would like to see it in for 5.14 RC, so I agree it should be merged.

@seamuslee001
Copy link
Contributor

@colemanw can you rebase against 5.14 in that case then

@colemanw colemanw changed the base branch from master to 5.14 May 2, 2019 21:37
@civibot civibot bot added 5.14 and removed master labels May 2, 2019
@seamuslee001
Copy link
Contributor

Added merge on pass based on Kevin's comments

@seamuslee001
Copy link
Contributor

Merging

@seamuslee001 seamuslee001 merged commit bfb1397 into civicrm:5.14 May 2, 2019
@colemanw colemanw deleted the menubarVars branch May 2, 2019 23:43
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.

3 participants