[WIP] Fix font color and ownership color clash for better usability

README.md

@@ -3,6 +3,8 @@
 ![Demo](demo.gif)
 
 Adds a font color picker to the Etherpad toolbar, with HTML export support.
+If authorship colors are enabled, the chosen font color is still saved but
+the editor will warn that author colors are masking the preview.
 
 ## Install
 
@@ -26,4 +28,4 @@ To reposition the color picker in the toolbar, add `fontColor` to your `settings
 
 ## License
 
-Apache-2.0
+Apache-2.0

static/js/index.js

@@ -3,15 +3,46 @@
 const {inlineAttribute} = require('ep_plugin_helpers/attributes');
 
 const colors = ['black', 'red', 'green', 'blue', 'yellow', 'orange'];
+const authorColorsMessageTitle = 'Font colors are hidden by authorship colors';
+const authorColorsMessageText =
+    'Your font color was saved. Turn off "Colors" in Settings to preview it.';
+let authorColorsNoticeShown = false;
 
 const fontColor = inlineAttribute({attr: 'color', values: colors});
 
 exports.aceAttribsToClasses = fontColor.aceAttribsToClasses;
 exports.aceCreateDomLine = fontColor.aceCreateDomLine;
 
+const hasAuthorColorsEnabled = () => $('iframe[name="ace_outer"]').contents()
+    .find('iframe[name="ace_inner"]').contents().find('#innerdocbody').hasClass('authorColors');
+
+const getInnerDocBody = () => $('iframe[name="ace_outer"]').contents()
+    .find('iframe[name="ace_inner"]').contents().find('#innerdocbody').get(0);
+
+const updateToolbarHint = () => {
+  const authorColorsEnabled = hasAuthorColorsEnabled();
+  const hint = authorColorsEnabled ? authorColorsMessageText : '';
+  $('.font-color-icon a, .color-selection, #color-selection').attr('title', hint);
+  if (!authorColorsEnabled) authorColorsNoticeShown = false;
+};
+
+const maybeShowAuthorColorsNotice = () => {
+  if (!hasAuthorColorsEnabled() || authorColorsNoticeShown || !$.gritter) return;
+  authorColorsNoticeShown = true;
+  $.gritter.add({
+    title: authorColorsMessageTitle,
+    text: authorColorsMessageText,
+  });
+};
+
 // Bind the event handler to the toolbar buttons
 exports.postAceInit = (hook, context) => {
   const hs = $('.color-selection, #color-selection');
+  const scheduleHintRefresh = () => {
+    [0, 50, 150, 400, 800].forEach((delay) => setTimeout(() => {
+      updateToolbarHint();
+    }, delay));
+  };
   hs.on('change', function () {
     const value = $(this).val();
     const intValue = parseInt(value, 10);
@@ -21,6 +52,7 @@ exports.postAceInit = (hook, context) => {
       }, 'insertColor', true);
       hs.val('dummy');
       context.ace.focus();
+      maybeShowAuthorColorsNotice();
     }
   });
   $('.font_color').hover(() => {
@@ -31,6 +63,15 @@ exports.postAceInit = (hook, context) => {
     $('#font-color').toggle();
     context.ace.focus();
   });
+  const innerDocBody = getInnerDocBody();
+  if (innerDocBody && typeof MutationObserver !== 'undefined') {
+    new MutationObserver((mutations) => {
+      if (mutations.some((mutation) => mutation.attributeName === 'class')) updateToolbarHint();
+    }).observe(innerDocBody, {attributes: true, attributeFilter: ['class']});
+  }
+  $('#options-colorscheck, #padsettings-options-colorscheck, ' +
+      'label[for="options-colorscheck"], label[for="padsettings-options-colorscheck"]')
+      .on('click change', scheduleHintRefresh);
   // Re-render the niceSelect dropdown whenever the active UI language
   // changes. html10n rewrites the underlying <select>'s option text in
   // place, but niceSelect renders into its own DOM tree at init time and
@@ -41,6 +82,7 @@ exports.postAceInit = (hook, context) => {
       hs.niceSelect('update');
     });
   }
+  updateToolbarHint();
 };
 
 const doInsertColors = function (level) {
@@ -83,6 +125,7 @@ exports.aceEditEvent = (hook, call) => {
 
   if (cs.type === 'setBaseText' || cs.type === 'setup') return;
   setTimeout(() => {
+    updateToolbarHint();
     const colorSelect = $('.color-selection, #color-selection');
     colorSelect.val('dummy');
     colorSelect.niceSelect('update');

static/tests/frontend-new/specs/font_color.spec.ts

@@ -1,14 +1,16 @@
 import {expect, test} from '@playwright/test';
 import {clearPadContent, getPadBody, goToNewPad, writeToPad}
     from 'ep_etherpad-lite/tests/frontend-new/helper/padHelper';
+import {hideSettings, showSettings} from 'ep_etherpad-lite/tests/frontend-new/helper/settingsHelper';
 
 test.beforeEach(async ({page}) => {
   await goToNewPad(page);
 });
 
 const setColor = async (page: any, value: string) => {
-  await page.evaluate((v: string) => {
-    const sel = document.querySelector<HTMLSelectElement>('.color-selection')!;
+  const select = page.locator('#font-color select.color-selection');
+  await expect(select).toBeAttached();
+  await select.evaluate((sel: HTMLSelectElement, v: string) => {
     sel.value = v;
     sel.dispatchEvent(new Event('change', {bubbles: true}));
   }, value);
@@ -50,6 +52,26 @@ test.describe('ep_font_color', () => {
         () => document.querySelector<HTMLSelectElement>('.color-selection')!.value),
     {timeout: 5_000}).toBe('1');
   });
+
+  test('explains that authorship colors hide the chosen font color', async ({page}) => {
+    const padBody = await getPadBody(page);
+    await padBody.click();
+    await clearPadContent(page);
+    await writeToPad(page, 'foo');
+    await page.keyboard.press('ControlOrMeta+A');
+
+    await setColor(page, '1');
+    await expect(page.locator('.gritter-item').first())
+        .toContainText('Turn off "Colors" in Settings to preview it.');
+
+    await showSettings(page);
+    await page.locator('label[for="options-colorscheck"]').click();
+    await hideSettings(page);
+    await expect(page.frameLocator('iframe[name="ace_outer"]').frameLocator('iframe[name="ace_inner"]')
+        .locator('#innerdocbody')).not.toHaveClass(/authorColors/);
+    await padBody.click();
+    await expect(page.locator('#font-color select.color-selection')).toHaveAttribute('title', '');
+  });
 });
 
 test.describe('ep_font_color l10n', () => {