Now I'm actually building some stylized themes with Drupal 7 one thing I have run into strait away is that its very easy to break contextual links because they can inherit font-size.

I suggest we use the same approach as we did with Toolbar and add a keyword insulator.

CommentFileSizeAuthor
#4 contextual-links-font-size-insulator_2.patch706 bytesJeff Burnz
contextual-links-font-size-insulator.patch706 bytesJeff Burnz

Comments

eustace’s picture

eustace commented

The rationale behind the changes are clear and I vote for the changes. However, why change the font-size specification from "90%" to "small" in "div.contextual-links-wrapper"? While both styles use relative sizing, it appears the lower granularity in using named specifiers like "small" is more liable to cross browser inconsistencies. How about an em sizing to be more consistent with referenced Toolbar?

Jeff Burnz’s picture

Jeff Burnz commented

You can't use a relative unit as an insulator - that's the whole entire issue right there and why I posted this as a bug.

Keywords are not relative units, they are absolute units, their relativity is only to the browser default - this is quite different to how ems, ex, or % work. Cross browser consistency is simply irrelevant - we're talking fractional pixels at most.

FYI I wrote the Toolbar font size insulator patch and I can tell you it does not use ems as the insulator, it uses small, then sets all the font sizes relative to small using ems, which is how this patch works also.

sun’s picture

sun
Primary language German
Location Karlsruhe
commented
Status: Needs review » Needs work

Nice trick.

+++ modules/contextual/contextual.css	28 Sep 2010 14:05:11 -0000
@@ -61,6 +61,7 @@
+  font-size: 0.923em;

Can we either use 0.9em or use a different unit? Three decimal digits are just weird.

Powered by Dreditor.

Jeff Burnz’s picture

Jeff Burnz commented
Status: Needs work » Needs review
StatusFileSize
new contextual-links-font-size-insulator_2.patch706 bytes

Actually there is method to the madness, small x 0.923em = 12px (exactly, assuming a 16px default browser setting).

If we want something more comprehensible we can use medium x 0.75em = 12px.

0.9 of small is just 11.7px, so we have fractional px to deal and inconsistent results across browser due to how different browsers handle fractional pixels. I prefer to stick to a proper font scale 12px = 9pt and avoid fractional issues. Sorry for the long winded explanation, as a designer these are thing one obsesses over...

Status: Needs review » Needs work

The last submitted patch, contextual-links-font-size-insulator_2.patch, failed testing.

casey’s picture

casey commented

Default CSS could use "font: menu;"

http://www.w3.org/TR/CSS2/fonts.html#font-shorthand
The following values refer to system fonts:
[...]
menu The font used in menus (e.g., dropdown menus and menu lists).
[..]

geek-merlin’s picture

geek-merlin
Primary language German
Location Freiburg, Germany
commented
Version: 7.x-dev » 9.2.x-dev
Issue summary: View changes

@Jeff Burnz #4 Thanks! really enlightening.

Targeting D8 (once this module is maintainer ;-/ as this is still an issue in D8 contextual.theme.css

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

larowlan’s picture

larowlan
Location 🇦🇺🏝.au GMT+10
commented
Status: Needs work » Closed (duplicate)
Issue tags: +Bug Smash Initiative

The fonts now use small (or large for touch enabled).

So we can close this one now