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

Menu bar - Allow customization. Restore default colors. #13996

Merged
merged 3 commits into from
Apr 9, 2019

Conversation

colemanw
Copy link
Member

@colemanw colemanw commented Apr 8, 2019

Overview

v5.12 introduced a new menu bar. The change also revised the default color scheme (applying a more contemporary palette), but this creates stress for some users who were trained to recognize the old menu color.

This exposes a new setting for customizing the color scheme, and it changes the default to match previous scheme.

Before

Menubar color hardcoded to #f2f2f2

After

Menubar color defaults to #1b1b1b (consistent with previous versions of CiviCRM) and is configurable via "Display Settings" screen:

image

@civibot
Copy link

civibot bot commented Apr 8, 2019

(Standard links)

@civibot civibot bot added the master label Apr 8, 2019
@colemanw
Copy link
Member Author

colemanw commented Apr 8, 2019

FYI @Stoob @totten @mattwire @agh1

@seamuslee001
Copy link
Contributor

@colemanw can you pls rebase this against current master pls

@colemanw colemanw force-pushed the menuColor branch 2 times, most recently from dda76b9 to 6a3e6d9 Compare April 8, 2019 04:02
settings/Core.setting.php Outdated Show resolved Hide resolved
@Stoob
Copy link
Contributor

Stoob commented Apr 8, 2019

The configurable color setting is fresh, thx @colemanw

@agh1 I don't have a large sample size of reactions to go on. But I think that defaulting to the old grey menu bar color is also a safer move.

But this is with the caveat that regardless of the default this cool feature should be combined with a popup that says "hey you can change your menu bar color now (link to config page, above)". Let's show this off a bit! This is akin to the experience I've seen in other software where it's says "hey would you like to try our new look"?

@MegaphoneJon
Copy link
Contributor

Personally, I'm mostly excited by the ability to put a toolbar color into civicrm.setting.php to differentiate my test sites more easily :)

@colemanw
Copy link
Member Author

colemanw commented Apr 8, 2019

Yep that is one upshot of this change :)
As @MegaphoneJon astutely observed, the menu color is now a setting, and settings can be overridden by your config files, and you can have different config files on e.g. your staging and test servers.

@colemanw
Copy link
Member Author

colemanw commented Apr 8, 2019

@agh1 I've added a validate callback to the setting so the api won't let you save anything that isn't a properly formed 7-character hex color.
@Stoob or @agh1 can you review this PR?

Ex: If an admin uses an API call (CLI/REST) to change the menubar color,
then they don't need to follow-up with a cache-clear.  The new setting just
goes live.

Ex: If a customization (via `civicrm.settings.php` or via extension) decides
on the color scheme programmatically (e.g.  per-domain or per-role or
per-user-preference), then they don't need to clear cache.  Multiple color
schemes can coexist.
@totten
Copy link
Member

totten commented Apr 9, 2019

(CiviCRM Review Template WORD-1.2)

  • General standards
    • (r-explain) Pass: Tweaked a bit to add context. The explanation is still a little cheeky, but that picture is so clever... :)
    • (r-user) Pass: Improves r-user for a recent enhancement.
    • (r-doc) Pass
    • (r-run) Pass: I locally tried it out using a mix of of production-mode/debug-mode and color-schemes, with changes made via web UI and CLI. All worked as expected.
  • Developer standards
    • (r-tech) Pass
    • (r-code) Pass: I have another very small tweak, but it can wait in a separate PR. More important to get this moving so we can branch the RC.
    • (r-maint) Pass
    • (r-test) Pass: There are reported failures, but these appear unrelated.

@totten totten merged commit f9e118b into civicrm:master Apr 9, 2019
@colemanw colemanw deleted the menuColor branch April 9, 2019 01:11
@totten totten changed the title Configurable menubar color Menu bar - Allow customization. Restore default colors. Apr 16, 2019
@zkrebs
Copy link

zkrebs commented May 16, 2019

I think the image should also reflect a male figure in a state of distress and also drop the lines about "inclusivity". I'd like to be able to forward this to my predominantly female coworkers who believe in GLBTQQ inclusivity, etc.

Sorry to be that person. I laughed, but I am also realistic that this could make someone dislike CiviCRM.

@Stoob
Copy link
Contributor

Stoob commented May 16, 2019

Ok @zkrebs by the way it was not the intent to have that image publicly published, originally posted it on the partners private channel. Come to think of it, perhaps it should be removed from this page @colemanw

@zkrebs
Copy link

zkrebs commented May 16, 2019

Understood. Thanks for everyone's hard work to get these features done.

@colemanw
Copy link
Member Author

colemanw commented May 16, 2019

It's the first time I've heard someone argue we need more male representation in this field, but sure. I've cleaned up the PR description to be more straightforward and removed the image, which was a joke and so probably not the most "professional" thing to put on a PR :)

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.

7 participants