Remove Open Sans?

Seems like this should be in the Theme layer somewhere. Perhaps explain more how this font is used and why it's hardcoded in Panopoly? Wouldn't other themes potentially use different fonts?

The according css is loaded for anonymous users, too.
I've removed it with a css unset for them, because we are using another font and it's a needless request.

+1 for making this part of the admin experience and configurable.

I support making this conditional as well. You would even need to remove Open Sans from the CSS itself, as the browser will just use the next font in the CSS-defined font-family stack. All you need to do is make loading the @font-face conditional optional.

Can anybody tell where this is hardcoded in Panopoly?

This is in panopoly_core_page_build() in panopoly_core.module.

Dare we just remove the code:

/**
 * Implemenets hook_page_build().
 */
function panopoly_core_page_build(&$page) {
  // This fixes a bug that causes @font-face declarations to break in IE6-8.
  // @see http://www.smashingmagazine.com/2012/07/11/avoiding-faux-weights-styles-...
  $path = drupal_get_path('module', 'panopoly_core');
  drupal_add_css($path . '/css/panopoly-fonts-ie-open-sans.css', array('group' => CSS_THEME, 'every_page' => TRUE, 'browsers' => array('IE' => 'lte IE 8', '!IE' => FALSE), 'preprocess' => FALSE));
  drupal_add_css($path . '/css/panopoly-fonts-ie-open-sans-bold.css', array('group' => CSS_THEME, 'every_page' => TRUE, 'browsers' => array('IE' => 'lte IE 8', '!IE' => FALSE), 'preprocess' => FALSE));
  drupal_add_css($path . '/css/panopoly-fonts-ie-open-sans-italic.css', array('group' => CSS_THEME, 'every_page' => TRUE, 'browsers' => array('IE' => 'lte IE 8', '!IE' => FALSE), 'preprocess' => FALSE));
  drupal_add_css($path . '/css/panopoly-fonts-ie-open-sans-bold-italic.css', array('group' => CSS_THEME, 'every_page' => TRUE, 'browsers' => array('IE' => 'lte IE 8', '!IE' => FALSE), 'preprocess' => FALSE));
}

This should really be in some kind of theme layer - and is there to fix issues with IE6-8. I am voting for removing it.

Patched worked, and I believe that this should be applied to Panopoly Core. Should be up to theme developers to handle fonts and any cross-browser rendering issues.

The patch isn't making it conditional, rather, just removes it outright.

The patch would be better suited (in my opinion) if we added a on/off form element in Panopoly's admin UI and they conditionally loaded the script in hook_page_build based on that value.

In my opinion, I don't think it makes sense to have a conditional option for this - it should just be removed.

If someone writes a theme that depends on Open Sans from panopoly_core and then it's disabled - it could get messed up. Or what if the site uses two different themes in different situations, and one depends on Open Sans and the other is broken by Open Sans?

The inclusion of fonts should be handled via the theme itself OR the site builder via a module like fontyourface (which can manage some pretty complex configurations). Personally, I think Panopoly should stay out of it!

Think I just saw a css file that should also be removed. Cannot remember exactly where, but will find it again.

I think it should be removed - or at least moved from core to theme.

http://drupalcode.org/project/panopoly_core.git/blob/refs/heads/7.x-1.x:... --> font.css included
http://drupalcode.org/project/panopoly_core.git/tree/refs/heads/7.x-1.x:... --> font directory included
http://drupalcode.org/project/panopoly_core.git/tree/refs/heads/7.x-1.x:... --> font css files

My major argument against removing it outright is Panopoly specifically markets itself as improving the administration UI--typography is absolutely a part of that.

Making it conditional puts minimal burden on site itself, without overly complicating the codebase. Thanks to font stacks in the css files themselves, not having Open Sans enabled will simply mean the browser will default to the next available font in the stack.

If the goal is to improve the admin UI, we could do that by bundling an admin theme that has Open Sans? Really, there's nothing wrong with Open Sans itself, but it'd be easier to deal with if it were added via a theme..

I agree that the distro / modules shouldn't provide the font. I think when we get to including Radix we could eventually offer an improved admin interface and that theme handles the font.

Removed everything.

that patch looks like it has a lot of other changes in it as well. Was it made against the latest dev release?

also @lsolesen try git config core.fileMode false to avoid permission changes ending up in the patch.
http://stackoverflow.com/questions/1580596/how-do-i-make-git-ignore-mode...

Echoing what #20 said. Seems patch was not rolled against dev.

Something strange indeed. Think i forgot to merge branches.

Now without other noise.

Also I do not think it should be included and it is not crucial part of any experience.

Should it be part of the Admin section then it should be in some customized Admin theme that does or does not rely on some other theme (Radix or any other). There are two concepts with that, making everything larger (for mobile experience to be able to use) and smaller (for laptop/desktop to be able to fit more information).

I think Admin theme should be on its own OR if some theme is going to be part of Panopoly (Radix as it seems planned) then lets base on it.

On the other hand, I'm not sure how much energy should go in there and how much in everything else, in particular more on Panels system integration like Panels everywhere and also quality of Widgets as they really add much more to overall experience.

I do oppose to having some conditional as that really makes little sense - make Panopoly much more SASS and leave that to SASS part of building. You cannot have powerful base and babysit users.

Yeah we should take a screenshot of the before and after of a few elements and make any line-height or other changes if necessary.

Though the/a admin theme /should/ be dealing with these style changes.

Tagging as such.

Here are some screenshots. Plus one for this because it ruins an admin theme I'm working on based on Material Design which utilizes Roboto.

Open Sans.

Replaced with just sans-serif.

Removing "Need Screenshots" tag.

@mglaman What does the screenshots show? Any difference?

The 'sans-serif' font on @mglaman's computer has a little less weight, and so appears a little smaller. But personally, I think it still looks pretty good! @populist: What do you think?

Yeah it was just a weight issue. However my concern style is within panopoly-modal.css in Widgets (I say make a "clean up CSS, remove fonts" issue once this issue is settled/committed.) I want that gone as well, because it's one more thing a theme developer will have to fight to override.

IMO if this is such a big issue it should be moved out of modules and a new admin theme for Panopoly built or a specific one chosen.

@mglaman Agreed :)

Screenshots seem perfectly fine to me, thanks @mglaman for making them! I did some of my own testing on the patch, and everything seemed totally acceptable to me. Committed!