From 6cbf7af041ea687e934d3629259ccfc4c6cfd9ab Mon Sep 17 00:00:00 2001
From: Robert Knight <robertknight@gmail.com>
Date: Tue, 23 Aug 2022 12:01:43 +0100
Subject: [PATCH] Replace `rem` units with a CSS variable for annotator font
 sizes

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 https://github.com/hypothesis/client/issues/4615
---
 src/annotator/index.js | 17 +++++++++++++++++
 tailwind.config.mjs    | 13 +++++++------
 2 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/src/annotator/index.js b/src/annotator/index.js
index d7dc21d85a0..661fc7b98dc 100644
--- a/src/annotator/index.js
+++ b/src/annotator/index.js
@@ -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.
@@ -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')
   );
diff --git a/tailwind.config.mjs b/tailwind.config.mjs
index adb9860f839..053a60e4b4b 100644
--- a/tailwind.config.mjs
+++ b/tailwind.config.mjs
@@ -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