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

Change Bartik default color to black #842

Closed
docwilmot opened this issue Apr 2, 2015 · 49 comments
Closed

Change Bartik default color to black #842

docwilmot opened this issue Apr 2, 2015 · 49 comments

Comments

@docwilmot
Copy link
Contributor

A couple of posts around the web about Drupalers checking out backdrop have expressed disappointment that the first impression is "this is just Drupal!"

I know we're to work on a new core theme eventually, but I propose, to dilute the let-down factor, we just make Bartik a very non-Drupal Black, just like the API site. That, with the new default node/1 would be a nice simple change, and wouldn't take much.

@quicksketch
Copy link
Member

I like this idea because the gradient in Bartik looks awfully dated. On the other hand, I'm not sure that many people would want a stark-block website. I'm curious what others think about this proposal.

@docwilmot
Copy link
Contributor Author

Or one of these from the presskit?

backdrop-colors

@docwilmot
Copy link
Contributor Author

black

blue

blue

green

green

grey

grey

red

red

@serundeputy
Copy link
Member

#000;

@pablo-romero
Copy link

I like black, too. But admin-bar loses too much contrast.
I would either tweak the default admin-bar css if using black, or use any other color for the header (e.g. grey header and black footer).

@docwilmot
Copy link
Contributor Author

Actually liking the blue now.

I think without the Bartik rounded tabs and without the gradient, it looks quite sufficiently non-Drupal, and looks quite modern/flat-design-y, and satisfies @quicksketch's concern about the acceptability of plain black, and @pablo-romero's concern about the admin bar.

@quicksketch
Copy link
Member

Yeah one item that modernizes the whole theme is if you make the header a flat color and remove the gradient. In our situation, that means just setting the top and bottom color to the same thing, and boom, more modern theme.

@docwilmot
Copy link
Contributor Author

Tried this. It seems that color module cant mix flat and gradient. I attempted to have a default with flat but keep the gradients in color.inc in case someone wanted this still. but It seems color module needs to be able to replace color1 and color2 (top and bottom in this case) from color.inc with the gradient colors in colors.css (maybe vice versa).

Trying to have two same colors causes all the color schemes to be non-gradient.

If anyone can figure this out, please advise, otherwise, shall we ditch gradients altogether and go flat for all schemes?

@docwilmot
Copy link
Contributor Author

Unchanged from above, essentially.

screenies

@quicksketch
Copy link
Member

I think we're going to have to find a more compatible way of implementing this change. I don't think we can remove the ability to have a gradient at all mid-release. Even though we might not be affecting very many existing sites, this wouldn't set a good precedent.

Trying to have two same colors causes all the color schemes to be non-gradient.

Ahhh, I think I see what you're saying. The way color module works is replacing the "default" color with the selected color from the scheme. If the two default colors are the same value, they both get replaced twice, resulting in only one color showing.

@quicksketch
Copy link
Member

What if we left the default color.css exactly as-is and just set a default color scheme during the default profile installation?

The color module API is not very good. I already separated the CSS generation in the existing issue at #792, but it's waiting on tests. After that is complete, we could just call the new color_save_configuration() function added in that issue.

@quicksketch
Copy link
Member

and just set a default color scheme during the default profile installation?

Which by-the-way, I think we should provide a set of less-dated color schemes as well.

Went ahead and made a PR to change Bartik to use flat colors and no tabs.

We may need to find a way to make the removal of tabs compatible as well. I see the current PR makes a new color for "Active menu item" (good idea), maybe we should make another one for "Menu item"? That's definitely making a lot of options (and a lot of rope for users to hang themselves with bad color schemes).

@docwilmot
Copy link
Contributor Author

What if we renamed this PR effort as jen_bartik, (or quick_bartik, or even bartik_sketch) and put bartik in contrib?

Or how about 'nearly flat' with a gradient of #27ace2 to #27ace1?

What if we left the default color.css exactly as-is and just set a default color scheme during the default profile installation?

Would this default need to be a gradient still or will we be able to make a non-default two-same-color 'gradient' and save that as default?

Frankly if we had such tags, I'd tag this critical; I'd be wrong, but it would demonstrate how much I wish we didnt ship with default bartik.

@quicksketch
Copy link
Member

I'd be wrong, but it would demonstrate how much I wish we didnt ship with default bartik.

I'm not opposed (and I don't think anyone would be) to making a new default theme mid-cycle. As long as it's an additive process (i.e. Bartik sticks around in core for everyone using it). I think we should do the least amount of change possible to modernize what we have, and save any major change for a new default theme.

@Graham-72
Copy link

I am probably being awkward saying this, but as a Drupal user I found it strangely re-assuring to find trusty Bartik there in Backdrop! I felt I was in familiar territory. But I have never found Bartik's colour changing arrangement particularly inviting, and a 'Bartik plus' that offered a few check button options might be nice.

@docwilmot
Copy link
Contributor Author

How about a little subtheme - bartik_flat - with nothing but color and CSS changes?

@Graham-72
Copy link

Yes, and could it have a checkbox to choose whether to have main menu tab borders showing or not?

@quicksketch
Copy link
Member

Would this default need to be a gradient still or will we be able to make a non-default two-same-color 'gradient' and save that as default?

We can make the two colors the same to effectively remove the gradient. It's already possible to use the same color twice in a color scheme, it's just not possible to use the same color twice in the Bartik-provided "colors.css" file.

So the fix in this issue is simply to set the color scheme in standard.install that will immediately override the colors.css file in Bartik by saving a new palette. e.g. something like this:

$themes = list_themes();
$theme_info = $themes['bartik'];
$palette = array(
  'top' => '#48a9e4',
  'bottom' => '#48a9e4',
  'bg' => '#ffffff',
  'sidebar' => '#f6f6f2',
  'sidebarborders' => '#f9f9f9',
  'footer' => '#292929',
  'titleslogan' => '#fffeff',
  'text' => '#3b3b3b',
  'link' => '#0071B3',
);
color_save_configuration('bartik', $theme_info, $palette)

This could probably be made a bit cleaner, the color_save_configuration() function doesn't exist yet, it's part of the bug fix in #792.

@docwilmot
Copy link
Contributor Author

I understand and I think that's good enough for now; at least better by far than the old gradient. Would have been nice to be able to offer the user more than one flat-color option though. On which note I have one last option, then I'll stop being annoying. Have a look at this; its a gradient from #48a9e4 to #48a9e3, and I find the difference imperceptible. Is it acceptable to cheat like that?:

compare

We may need to find a way to make the removal of tabs compatible as well

Can we do this mid-cycle too? A theme setting to restore tabs using CSS maybe?

If all above fails, I'm going to put Bartik Flat as a contrib subtheme after renaming it something less goofy.

@quicksketch
Copy link
Member

Would have been nice to be able to offer the user more than one flat-color option though.

I think we can still provide the user multiple flat-color options as different color schemes in the UI. I haven't looked into how this works, but I think it's possible.

Have a look at this; its a gradient from #48a9e4 to #48a9e3, and I find the difference imperceptible. Is it acceptable to cheat like that?

I think it's acceptable.

A theme setting to restore tabs using CSS maybe?

This could be added via a setting mid-cycle yes, though I'd like to get others' feedback on it.

@docwilmot
Copy link
Contributor Author

I think we can still provide the user multiple flat-color options as different color schemes in the UI. I haven't looked into how this works, but I think it's possible.

You're absolutely right, I just checked. As long as the default color scheme has two gradient colors, it all works fine. The others can have the same hex code for the 'top' and 'bottom' gradient color, no problem.

So we can set the default to be a near gradient (#27ace2 to #27ace1 for example) and/or set the default using color_save_configuration(), and then set the other color schemes to non-gradient (and/or keep some gradients for those so inclined).

So just the tabs to get an opinion on.

@serundeputy @pablo-romero @Graham-72 @klonos any thoughts on

  • if we should remove tabs from Bartik now, being mid-cycle as we are now
  • if yes, we would need to keep the ability for users to restore tabs if they prefer them; how?

@klonos
Copy link
Member

klonos commented Apr 28, 2015

I rarely use Bartik. So, not leaning towards one way or another specifically, but I am all for progress/improvement even if that means to "sacrifice" backwards compatibility when it comes to themes. Functionality (i.e. modules and core) is another story.

If we go with this, then a UI option would be best I believe.

@docwilmot: Did I say well done on coming up with the near gradient solution? Good thinking!

@pablo-romero
Copy link

@docwilmot I don't mind keeping the tabs as long as we keep Bartik, until there's a new default front-end theme. I never use Bartik either (except for some local tests).

Instead of removing the tabs, I'd just "modernize" them a little bit, and make them degrade better on small screens (css only). There's also an issue @BWPanda wrote about sub-items at #726.

@docwilmot
Copy link
Contributor Author

Thanks @pablo-romero; latest try should please most people:

  • leaves all the original options in Bartik available
  • plus, installs with a basic flat color header
  • plus adds a toggle to turn tabs on and off'
  • plus adds a couple new theme color options

@quicksketch
Copy link
Member

Thanks @docwilmot! I gave the PR a spin. I ran into a few issues:

  • The PR contains a few tabs instead of spaces.
  • We probably need an upgrade path to set the "Main menu tab style" setting, or we need to assume an empty value means "rounded corners". I went to the bartik settings page and saved it without making any changes and I lost the rounded corner look because no default value was set.
  • The white text links on the main navigation are difficult to read on the default color scheme when tabs are enabled. The text used to be black on the tabs, but now it's white:

Old:
bartik-previous

New:
bartik-new

I love the new schemes "Giant Goldfish", "Mocha", and "Black Drop". "Icier" seems too similar to "Ice". Maybe we just update "Ice" directly? I'd love to drop "Firehouse" entirely. One nice thing about the way that schemes are saved is that the names are irrelevant in the config storage, only the colors are stored. So if we delete or update schemes, it won't impact existing sites other than to indicate that the scheme is "Custom" instead of a preset.

@klonos
Copy link
Member

klonos commented Dec 14, 2015

How about having a white 1 or 2px outline around Drop?

@docwilmot
Copy link
Contributor Author

Better yet a (negative) reverse-color version for dark backgrounds.

@jenlampton
Copy link
Member

Thanks for all the progress on this issue :)

For starters, I prefer the black color to the blue from a branding perspective. I don't think the disappearing dragon is an issue because I don't think we should include a logo (or dragon) in a front-end theme . It always bothered me that when you used Drupal to build your OWN website, you ended up with a blue alien-head staring back at you. The logo / branding should belong to the owner of the site.

I think the tabs changes are out of scope for this issue, and may slow us down (I found myself wanting to make suggestions about better ways to handle that code on the PR). Can we just table anything that's not the requested color change, and do the other things as a follow-up? I'd like to focus on getting this in for 1.3.0.

@jenlampton
Copy link
Member

Well I just talked it over with @quicksketch and he filled me in on why we need the tab thing to be a setting (because we want to remove the rounded-ness, but we can't go changing the way people's sites look if they are actually using Bartik). So I get it now! Let's leave the tabs.

So it looks like there's only three small changes needed for the PR:

  1. make Black the default color scheme (and remove the word Drop?)
  2. make sure there aren't two blue color schemes (remove blue drop and update blue lagoon with new colors)
  3. update hook to change a previous "default" setting to "Blue lagoon".

@klonos
Copy link
Member

klonos commented Jan 1, 2016

For starters, I prefer the black color to the blue from a branding perspective.

I bet that's what most of us were thinking too.

Now, as for the logo, I don't really mind it. I always thought of it as a placeholder and in my head I'd imagine how the site would look with its own logo. With layouts in core and what we are planning for blocks, I'd expect the logo to live in its own "Logo" block that people would be able to move around to where they want it (left/right of their site name/slogan).

Loosing the rounded corners of the tabs brings the theme to the 21st century with a more "material" design. Plain as that.

@jenlampton
Copy link
Member

I don't think I've seen a layout yet that lets you move the logo from left to right. The logo is uploaded under "Site settings" (where you add your site name) and there's a checkbox on the header block allowing you to show it (or not). I suppose a contrib module could add a new header block that's actually more like a mini-panel and lets you arrange the things inside it, but the only way to solve that problem today is with css in the theme. But now that I've written all this, I don't think it's relevant to this issue :)

@docwilmot
Copy link
Contributor Author

  1. update hook to change a previous "default" setting to "Blue lagoon".

How do you do an update hook for a theme?

@docwilmot
Copy link
Contributor Author

Never mind above.

New question: if we save Blue lagoon with new colors, that will change people's site appearance on update, which we shouldn't do. So my plan was to save Bartik's current color scheme as a 'custom' palette on upgrade. Is this reasonable?

To do that though, there is no simple function which saves color palettes. Its all done in color_scheme_form_submit() and that only changes the $form_state values so that the real saving is done in system_theme_settings_submit().

So to save a custom palette I'd need to directly create a new color folder infiles and save a copy of the contents of the default Bartik CSS file in there, then save the path to that in bartik.settings.json as usual.

But, like any other custom palette if the user decides to try out a new palette, the custom settings would be lost and gone forever.

Alternatively, in conclusion, to ensure that the old Bartik color scheme is available always, we could keep Blue lagoon AND the new flat blue (which I called Blue Drop).

Long post for little gain I suspect. Whats the way to go here?

@docwilmot
Copy link
Contributor Author

I need some help with this.

  1. update hook to change a previous "default" setting to "Blue lagoon".

I cant think of anyway to do this sanely. The theme settings config .json doesnt save with a key, only with a color palette. On viewing the theme settings page, the current palette is compared with the existing theme palettes and if none match, it is labelled as custom. And when you save, it generates a folder name from an MD5 of the palette and saves the new CSS theere, then saves that path in the config.

So I cannot simply save a previously default setting as blue lagoon. Here is my try:

function system_update_1046() {
  $theme = config_get('system.core', 'theme_default');

  // Only bother if the current theme is Bartik.
  if ($theme == 'bartik') {
    // Define the old Blue Lagoon color palette
    $palette_default = array (
      'top' => '#0779bf',
      'bottom' => '#48a9e4',
      'bg' => '#ffffff',
      'sidebar' => '#f6f6f2',
      'sidebarborders' => '#f9f9f9',
      'footer' => '#292929',
      'titleslogan' => '#fffeff',
      'text' => '#3b3b3b',
      'link' => '#0071B3',
      'menu' => '#f3f7fa',
      'activemenu' => '#fefeff',
    );
    $palette = theme_get_setting('color.palette', 'bartik');
    // If there were no color settings, then Blue Lagoon was default
    if (empty($palette)) {
      $palette = $palette_default;
    }
    // _color_rewrite_stylesheet() requires a 'base' key
    $palette['base'] = '#ffffff';

    // Much of the following code is copied verbatim from 
    // color_scheme_form_submit() since there doesn't appear to be another way
    // to save a color palette.
    $info = color_get_info('bartik');
    $info['schemes'][''] = array('title' => t('Custom'), 'colors' => array());

    // Prepare target locations for generated files.
    $theme = 'bartik';
    $file = 'css/colors.css';
    $id = $theme . '-' . substr(hash('sha256', serialize($palette) . microtime()), 0, 8);
    $paths['color'] = 'public://color';
    $paths['target'] = $paths['color'] . '/' . $id;
    foreach ($paths as $path) {
      file_prepare_directory($path, FILE_CREATE_DIRECTORY);
    }
    $paths['target'] = $paths['target'] . '/';
    $paths['id'] = $id;
    $paths['source'] = backdrop_get_path('theme', $theme) . '/';
    $paths['files'] = $paths['map'] = array();

    $style = backdrop_load_stylesheet($paths['source'] . $file, FALSE);

    // Return the path to where this CSS file originated from, stripping
    // off the name of the file at the end of the path.
    $base = base_path() . dirname($paths['source'] . $file) . '/';
    _backdrop_build_css_path(NULL, $base);

    // Prefix all paths within this CSS file, ignoring absolute paths.
    $style = preg_replace_callback('/url\([\'"]?(?![a-z]+:|\/+)([^\'")]+)[\'"]?\)/i', '_backdrop_build_css_path', $style);

    // Rewrite stylesheet with new colors.
    $style = _color_rewrite_stylesheet($theme, $info, $paths, $palette, $style);
    $base_file = backdrop_basename($file);
    $css[] = $paths['target'] . $base_file;
    _color_save_stylesheet($paths['target'] . $base_file, $style, $paths);

    unset($palette['base']);

    // Now save the new theme setting to config
    $config_color_set = array(
      'theme' => 'bartik',
      'main_menu_tabs' => 'rounded-tabs',
      'op' => 'Save theme settings',
      'color' => array ( 
        'palette' => $palette,
        'stylesheets' => array (
          0 => $paths['target'] . 'colors.css',
        ),
        'files' => array (
          0 => $paths['target'] . 'colors.css',
        ),
      ),
    );

    $config = config('bartik.settings');
    $config->setData($config_color_set);
    $config->save();
  }
}

Works fine if you test this code block, but run as an update hook, its complaining Notice: Undefined index: base in _color_rewrite_stylesheet() because Bartik doesnt have a 'base' key in the color.inc file and the function needs one.

@docwilmot
Copy link
Contributor Author

Think I've got this fixed. PR soon.

@quicksketch
Copy link
Member

Looks like @jenlampton is working on rerolling this from backdrop/backdrop#1129.

@quicksketch
Copy link
Member

I rerolled @jenlampton's PR into a new one at backdrop/backdrop#1194.

This adopts the approach of just shipping with a "colors-legacy.css" file that contains the old default colors. However, I adjusted the way this stylesheet is selected. Instead of a checkbox for "Use legacy", we just use the Blue Lagoon option to detect the legacy mode

  • The Blue Lagoon color scheme is now back to it's original version (plus the two new colors for Menu Links and Active Menu Link).
  • Now if you select "Blue Lagoon" on the color settings form, it will prevent Color module from doing anything. Instead, a "color_legacy" flag is set in the bartik.settings.json config file.
  • In template.php, if this flag is set, we swap out colors.css with colors-legacy.css.
  • The upgrade path sets the Menu Style to "Rounded" and sets this "color_legacy" flag.

So now users can toggle back and forth between using the legacy CSS or not just by selecting Blue Lagoon. The other presets actually generate new color CSS files as they did previously. Unfortunately because themes do not have the ability to alter forms, this code lives in Color module, until we can remove it in 2.x.

I'd like to add tests before this is merged, but I'd love to get feedback from @jenlampton and @docwilmot. If it sounds good, either of you up for writing tests?

@quicksketch
Copy link
Member

If it sounds good, either of you up for writing tests?

Nevermind, after getting into the code tests are pretty easy. Updated the PR with a second commit with tests. backdrop/backdrop#1194

@quicksketch
Copy link
Member

Merged in backdrop/backdrop#1194 into 1.x for 1.3.0. Thanks @docwilmot for pioneering this and @jenlampton for improving it!

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

Successfully merging a pull request may close this issue.

8 participants