Skip to content

Commit

Permalink
Replace rem units with a CSS variable for annotator font sizes
Browse files Browse the repository at this point in the history
Various annotator UI components use rem-based font sizes to try and make the
annotator UI match the document. However various sites (eg. Stack Overflow,
OpenStax) set the root font size to a value that results in the Hypothesis UI
elements having unreadably small or excessively large text. Also for these sites
the rem-size doesn't reflect the actual "default" font size of the page as
perceived by the user.

To resolve this, change the font sizes from using rem units to a value
calculated from a `--hypothesis-font-size` CSS variable. This variable is set to
match 1rem when that lies in a reasonable range (16-24px) but is clamped to lie
within that range otherwise. In future we could change the way the reference
point is computed to try and match the "default" font size on the page, if that
is not 1rem.

Fixes #4615
  • Loading branch information
robertknight committed Aug 23, 2022
1 parent 97c4f12 commit 6cbf7af
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 6 deletions.
17 changes: 17 additions & 0 deletions src/annotator/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,21 @@ const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
* @typedef {import('./sidebar').SidebarContainerConfig} SidebarContainerConfig
*/

/**
* Set the `--hypothesis-font-size` variable that is used as a reference point
* for text which should scale to match the document's font size.
*
* In documents which set "reasonable" font sizes on the HTML element, this
* is the same as `1rem`. To prevent Hypothesis text from being too small/large,
* we constrain the min/max font size. See https://github.com/hypothesis/client/issues/4615.
*/
function setRootFontSize() {
const htmlEl = document.documentElement;
const documentFontSize = parseInt(getComputedStyle(htmlEl).fontSize);
const fontSize = Math.max(16, Math.min(documentFontSize, 24));
htmlEl.style.setProperty('--hypothesis-font-size', `${fontSize}px`);
}

/**
* Entry point for the part of the Hypothesis client that runs in the page being
* annotated.
Expand All @@ -55,6 +70,8 @@ const sidebarLinkElement = /** @type {HTMLLinkElement} */ (
* client is initially loaded, is also the only guest frame.
*/
function init() {
setRootFontSize();

const annotatorConfig = /** @type {GuestConfig & InjectConfig} */ (
getConfig('annotator')
);
Expand Down
13 changes: 7 additions & 6 deletions tailwind.config.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -88,12 +88,13 @@ export default {
xl: ['16px'],
'2xl': ['18px'],
'touch-base': ['16px', '1.4'], // Used for touch interfaces in certain UIs
// rem-based font sizes for annotator controls that should scale
// with text scaling in the underlying document
'annotator-sm': ['0.75rem'],
'annotator-base': ['0.875rem'],
'annotator-lg': ['1rem'],
'annotator-xl': ['1.125rem'],
// Font sizes for annotator controls that should scale with text in the
// underlying document. The `--hypothesis-font-size` var is initialized
// by the annotator bundle.
'annotator-sm': ['calc(0.75 * var(--hypothesis-font-size))'],
'annotator-base': ['calc(0.875 * var(--hypothesis-font-size))'],
'annotator-lg': ['var(--hypothesis-font-size)'],
'annotator-xl': ['calc(1.125 * var(--hypothesis-font-size))'],
// These are known cases when we want absolute sizing for fonts so
// that they do not scale, for example annotator components that are
// rendered next to the sidebar (which doesn't scale with host root
Expand Down

0 comments on commit 6cbf7af

Please sign in to comment.