feat(ux): show the add-comment button as disabled until text is selected (#96) (#435)

* feat(ux): show the add-comment button as disabled until text is selected (#96)

Commenting requires a selection; clicking the toolbar button with nothing
selected only popped a hint. Now the button is greyed (opacity + not-allowed
cursor, aria-disabled) whenever the selection is empty, and becomes active as
soon as text is selected — driven off aceEditEvent's rep.

Deliberately visual-only: the button stays clickable and still falls back to
the existing 'select text first' hint, so the primary action can never get
stuck disabled if a selection event is missed.

Pure plugin change — works on the latest release (2.7.3) and develop without a
core update. Adds a Playwright test that checks the state toggles
disabled→enabled→disabled as the selection changes.

Closes #96

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(#96): block add-comment activation while disabled + cache hot path

- Don't open the new-comment form when there is no selection: an
  aria-disabled control must not be activatable by mouse or keyboard.
- Cache the button collection and skip DOM work in updateAddCommentButtonState
  when the selection state is unchanged (aceEditEvent is a hot hook).
- Test: assert the disabled button does not open the form, and that it
  does once text is selected.

* fix(#96): guard add-comment on the live selection, not a stale flag

The cached _addCommentDisabled flag lags a just-made selection (it's
updated by the async aceEditEvent), so guarding the click on it blocked
legitimate clicks made immediately after selecting — breaking the shared
addCommentToLine test helper and every comment-box test that uses it.
Check the live ace selection at click time instead, defaulting to allow
on any read failure (displayNewCommentForm still guards the empty case).

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

Author: JohnMcLear

static/css/comment.css

@@ -396,6 +396,14 @@ input.error, textarea.error {
   margin-right: 6px;
 }
 
+/* #96: visually mark the toolbar "Add comment" button as unavailable until
+   text is selected (commenting needs a selection). It is also not activatable
+   while disabled (see the click guard in static/js/index.js). */
+.addComment.comment-btn-disabled {
+  opacity: .4;
+  cursor: not-allowed;
+}
+
 /* OTHER */
 .hidden {
   display: none;

static/js/index.js

@@ -162,8 +162,21 @@ EpComments.prototype.init = async function () {
   // On click comment icon toolbar
   $('.addComment').on('click', (e) => {
     e.preventDefault(); // stops focus from being lost
+    // An aria-disabled control must not be activatable by mouse or keyboard
+    // (keyboard Enter/Space on the link also fires click), so don't open the
+    // form while there is no selection (#96). Check the LIVE selection rather
+    // than the cached disabled-state, which can lag a just-made selection and
+    // would wrongly block a valid click.
+    if (!this.hasSelectionForComment()) return;
     this.displayNewCommentForm();
   });
+  // Start disabled-looking: nothing is selected on load (#96). aceEditEvent
+  // keeps it in sync from here on.
+  this.$addCommentButtons = $('.addComment');
+  this._addCommentDisabled = true;
+  this._lastHasSelection = false;
+  this.$addCommentButtons.addClass('comment-btn-disabled').attr('aria-disabled', 'true');
+
   // #8: don't offer commenting to read-only viewers unless an admin enabled it.
   if (clientVars.readonly && !clientVars.allowReadonlyComments) {
     $('.addComment').hide();
@@ -1043,6 +1056,27 @@ const getXYOffsetOfRep = (rep) => {
   }
 };
 
+// #96: keep the toolbar "Add comment" button's state in sync with the
+// selection — commenting requires a non-empty selection. While disabled the
+// button is also not activatable (see the click guard above).
+EpComments.prototype.updateAddCommentButtonState = function (rep) {
+  if (!rep || !rep.selStart || !rep.selEnd) return;
+  const hasSelection =
+    rep.selStart[0] !== rep.selEnd[0] || rep.selStart[1] !== rep.selEnd[1];
+  // aceEditEvent is a hot hook; skip the DOM query/writes when the selection
+  // state hasn't changed since the last event (#96).
+  if (hasSelection === this._lastHasSelection) return;
+  this._lastHasSelection = hasSelection;
+  this._addCommentDisabled = !hasSelection;
+  // Reuse the cached button collection; re-query once if it wasn't ready yet.
+  if (!this.$addCommentButtons || !this.$addCommentButtons.length) {
+    this.$addCommentButtons = $('.addComment');
+  }
+  this.$addCommentButtons
+      .toggleClass('comment-btn-disabled', !hasSelection)
+      .attr('aria-disabled', String(!hasSelection));
+}
+
 // #95: floating "add comment" button anchored to the current selection.
 // Lazily build the element once, in the same container the new-comment popup
 // uses (#editorcontainerbox), so it shares the popup's coordinate space.
@@ -1236,6 +1270,26 @@ EpComments.prototype.navigateComment = function ($el, dir) {
   }
 };
 
+// True when there is a non-collapsed selection right now. Read live from ace so
+// the add-comment activation guard (#96) reflects the actual selection rather
+// than the cached, possibly-stale, disabled-state flag.
+EpComments.prototype.hasSelectionForComment = function () {
+  // Default to allowing: displayNewCommentForm reads the selection itself and
+  // bails (with a hint) when empty, so only block here when we affirmatively
+  // observe a collapsed selection — never on a read failure (#96).
+  let hasSelection = true;
+  try {
+    this.ace.callWithAce((ace) => {
+      const rep = ace.ace_getRep();
+      if (rep && rep.selStart && rep.selEnd) {
+        hasSelection =
+          rep.selStart[0] !== rep.selEnd[0] || rep.selStart[1] !== rep.selEnd[1];
+      }
+    }, 'hasSelectionForComment', true);
+  } catch (err) { hasSelection = true; }
+  return hasSelection;
+};
+
 EpComments.prototype.displayNewCommentForm = function () {
   this.hideFloatingAddCommentButton();
   const rep = {};
@@ -1648,10 +1702,11 @@ const hooks = {
 
     if (['setup', 'setBaseText', 'importText'].includes(eventType)) return;
 
-    // Show/position the floating "add comment" button next to the current
-    // selection (#95). aceEditEvent fires on selection changes, so this keeps
-    // the button anchored to the text and hidden when nothing is selected.
+    // aceEditEvent fires on cursor/selection changes — keep both selection-driven
+    // affordances current: the toolbar comment button's enabled-look (#96) and
+    // the floating add-comment button anchored to the selection (#95).
     if (pad.plugins.ep_comments_page) {
+      pad.plugins.ep_comments_page.updateAddCommentButtonState(context.rep);
       pad.plugins.ep_comments_page.updateFloatingAddCommentButton(context.rep);
     }
 

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

@@ -0,0 +1,74 @@
+import {expect, test} from '@playwright/test';
+import {getPadBody} from 'ep_etherpad-lite/tests/frontend-new/helper/padHelper';
+import {aNewCommentsPad} from '../helper/comments';
+
+// #96: the toolbar "Add comment" button should look unavailable when there is
+// no selection (commenting needs selected text) and become available once text
+// is selected. While disabled it must also not be activatable (an aria-disabled
+// control must not open the form on click/keyboard). This spec checks the visual
+// state toggles with the selection AND that activation is blocked while disabled.
+test.describe('ep_comments_page - Add-comment button state (#96)', () => {
+  test('button is disabled-looking with no selection, enabled with one',
+      async ({page}) => {
+        test.setTimeout(60_000);
+        await aNewCommentsPad(page);
+        const inner = await getPadBody(page);
+        const addBtn = page.locator('.addComment');
+
+        await inner.click();
+        await page.keyboard.press('Control+A');
+        await page.keyboard.press('Delete');
+        await page.keyboard.type('hello world');
+
+        // After typing, the caret is collapsed → button looks disabled.
+        await expect.poll(async () =>
+          addBtn.first().evaluate((el) =>
+            el.classList.contains('comment-btn-disabled'))).toBe(true);
+
+        // Select all → button becomes available.
+        await page.keyboard.press('Control+A');
+        await expect.poll(async () =>
+          addBtn.first().evaluate((el) =>
+            el.classList.contains('comment-btn-disabled'))).toBe(false);
+        expect(await addBtn.first().getAttribute('aria-disabled')).toBe('false');
+
+        // Collapse the selection again → back to disabled-looking.
+        await page.keyboard.press('ArrowRight');
+        await expect.poll(async () =>
+          addBtn.first().evaluate((el) =>
+            el.classList.contains('comment-btn-disabled'))).toBe(true);
+      });
+
+  test('clicking the disabled button does not open the new-comment form',
+      async ({page}) => {
+        test.setTimeout(60_000);
+        await aNewCommentsPad(page);
+        const inner = await getPadBody(page);
+        const addBtn = page.locator('.addComment');
+        const newCommentForm = page.locator('.new-comment-popup, #newComment');
+
+        await inner.click();
+        await page.keyboard.press('Control+A');
+        await page.keyboard.press('Delete');
+        await page.keyboard.type('hello world');
+        await page.keyboard.press('ArrowRight'); // collapse selection
+
+        // Disabled (no selection).
+        await expect.poll(async () =>
+          addBtn.first().evaluate((el) =>
+            el.classList.contains('comment-btn-disabled'))).toBe(true);
+
+        // Activating it must NOT reveal the new-comment form.
+        await addBtn.first().dispatchEvent('click');
+        await page.waitForTimeout(500);
+        await expect(newCommentForm).toBeHidden();
+
+        // With a selection the same activation DOES open the form.
+        await page.keyboard.press('Control+A');
+        await expect.poll(async () =>
+          addBtn.first().evaluate((el) =>
+            el.classList.contains('comment-btn-disabled'))).toBe(false);
+        await addBtn.first().dispatchEvent('click');
+        await expect(newCommentForm.first()).toBeVisible({timeout: 5_000});
+      });
+});