Copy Button Modal Overhaul - #406
Open
nathanvercaemert wants to merge 23 commits into
Open
Conversation
* Copy Selection Highlight POC (#363) * Add selection info section to copy modal Track browser text selection via selectionchange events and display the highlighted verse range, version, and text snippet in the copy modal. Handles all passage DOM variants (Bible, commentary, interlinear, interleaved/column comparison) and works across desktop and mobile by capturing selection data before modal focus clears it. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Preserve selection tracking through minification * Initialize selection tracking during page startup * Inline selection verse extraction in startup listener --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * Show all touched versions in copy selection modal Collect every touched version in the selection summary so compare selections list all visible versions. Known shortcoming: interlinear mode still uses heuristic DOM-based attribution, so version reporting there can be ambiguous and may still need a dedicated rule. * Remove selected text from copy modal handoff * Use passage selections in copy modal * Add note/xref extraction and improved verse matching to copy modal Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: multi-version notes/xrefs with inline markers in copy modal Enable notes and cross-references in the copy modal when multiple Bible versions are loaded. Previously blocked by a single-version guard. - Add _fetchNotesForVersions: per-version API fetch with version-labeled end-note sections (e.g. "Notes (ESV):", "Cross references (HCSB):") - Add _injectMarkersIntoClone: inject inline markers (a-ESV), (n1-HCSB) into DOM clone before text extraction, matching end-note IDs - Add _xrefLetter: sequential letter generator for consistent xref IDs across inline markers and end-notes - Fix _getOsisIdsForRange: use .versenumber elements (matching goCopy's clone trimming) instead of .verseLink to fix verse range mismatch in multi-version mode - Update _extractNotesFromHTML to use sequential xref letters matching the inline marker scheme - Update _buildChapterVerseTable to check all loaded versions for notes capability, not just the master version Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: move copy button to panel toolbar on mobile On mobile/narrow screens (< 768px), hide the navbar copy icon and show a new copy button in the passage panel toolbar, to the left of the settings cog. On desktop the navbar button remains unchanged. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat: per-panel copy dropdown (phases 1-4) behind feature flag Introduce a new per-panel .copyDropdown that mirrors the passage settings cog, covering the full copy flow in-dropdown: - selection mode: snapshot-resolved one-click copy button - grid mode: compact verse-range picker (click-start / click-end) - options strip: version checkboxes + notes/xrefs toggles - status rows: success, cooldown, rapid-copy warning, clipboard-denied, copy-error, unresolved-selection, no-versions The flow coexists with the classic #copyModal during rollout. Enable via ?copyDropdown=1 or localStorage.step.copyDropdownEnabled=true. Key pieces: - view_menu_copy.js: PassageCopyMenuView + step.copyDropdown singleton. Per-panel isolation, mutual exclusion, selection snapshot, listener gating so dropdown clicks do not clobber the underlying native selection. - copy_text.js: _uiSink abstraction so goCopy's three UI touchpoints (success / rapid-warning / no-versions) route through the dropdown instead of #bookchaptermodalbody when the dropdown sink is installed. _isVersionChecked helper and a new optional opts param on goCopy (wantNotes, wantXrefs, checkedVersionIndices) let the dropdown pass choices directly without needing hidden form inputs. - step.util.js: selectionchange listener gates collapse events while the dropdown is open so clicking inside it does not flip deselectedAt. copyModal() checks the flag and routes to the dropdown. - step_ready.js + step.util.js: construct PassageCopyMenuView alongside PassageMenuView for every passage panel. - start.jsp: .copyDropdown markup and script-tag registration. - pom.xml: bundle registration for the minified JS. - InteractiveBundle.properties: new i18n keys for dropdown strings. - copy_dropdown.scss: layout + range-selection styling, RTL and reduced-motion overrides, mobile bottom-sheet. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * refactor: retire classic copy modal (phase 5) The per-panel copy dropdown now replaces #copyModal end-to-end. Remove the feature flag and delete the classic modal's render pipeline. Deletions in copy_text.js (classic-modal-only): initVerseSelect, _displayVerses, _buildSelectionOnlyPanel, _switchToGridMode, _buildChapterVerseTable, _evaluateNotesAvailability _defaultSink (replaced with a no-op fallback inside _sink()) _lastSleepTime The global step.copyText._notesInDOM; single-version note detection is now an inline local var in goCopy. step.util.js: copyModal() drops the ~50 lines of modal DOM construction; it is a 3-line forwarder that clicks the active panel's .copyDropdownToggle. Kept exported for any bookmarked / external JS that invokes it. Removed "copyModal" from modalsRequireUnfreezeOfScroll. view_menu_copy.js lifecycle rewrite: The view now owns open/close directly rather than relying on Bootstrap's data-toggle="dropdown" data-api. Bootstrap's dropdown plugin double-binds click handlers when programmatic $.fn.dropdown("toggle") and declarative data-toggle usage mix, which manifested as open→immediate-close on the very first user click under phase-5 testing. Replaced with: onToggleClick → open() / close() .copyDropdown manages its own .open class document-level click.copyDropdownOutside listener (bound on a 0ms setTimeout so the opening click doesn't self-close it) Also removed: - feature-flag IIFE, isEnabled() method, all flag guards - the $(document).on(".panelCopyBtn", ...) redirect start.jsp: - Deleted <a class="panelCopyBtn"> (superseded by .copyDropdown which is responsive across all screen sizes). - Removed style="display:none" from .copyDropdown (always visible now). - Removed data-toggle="dropdown" + dropdown-toggle class from the toggle (view owns the click lifecycle). step_ready.js: clipboard-unavailable guard now hides .copyDropdown instead of .panelCopyBtn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: copy-selection modal classes for classic #copyModal fallback Mirror the .copySelection* styling already present in copy_dropdown.scss so the same layout applies when the selection-only flow is rendered inside #copyModal rather than the per-panel dropdown. Covers the container (.copySelectionOnly), range label (.copySelectionRange), primary action button (.copySelectionPrimary, 44px tap-target), escape hint (.copySelectionEscape) and clipboard fallback banner (.copySelectionFallback). Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> * style: panel-bar color + cleanup for copy dropdown Three refinements to the per-panel copy dropdown: - Selection mode: drop the redundant `<p class="copySelectionLabel">` (the range string is already in the button label) and remove the outer border around the row — let the bordered button stand on its own. - Grid mode: restructure footer as a flex row (Copy left, Back to selection right) and drop the inline `margin-right` on Back; gap is handled by the flex layout. Render order matches visual + tab order. - All three modal action buttons (.copySelectionPrimary, .copyGridPrimary, .copyBackToSelection) now use --clrStrongText for their resting color to match the bible / reference / search panel-bar buttons. Border, padding, hover, and disabled state are still inherited from .copyPrimaryBtn. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com> --------- Co-authored-by: vercaemert <nvercaemert@gmail.com> Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
sort history before delete or use for redirect
Sync Master
Replace the top status row with a dedicated inline banner so the "text is copied" confirmation appears where the user is looking: - Selection mode: below the options strip (.copyBottomSuccess) - Grid mode: in the footer to the right of Copy, or filling the gap between Copy and Back-to-selection when present (.copyFooterSuccess) Banner is cleared on dropdown close, on _update, and defensively when _onRapidWarning fires so a stale green can't coexist with the cooldown warning at the top. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PassageMenuView._initUI (view_menu_passage.js:342-343) removes any descendant matching .dropdown-menu.pull-right.stepModalFgBg from the shared .passageOptionsGroup on its first run, and our .copyMenu carries those exact classes. After first cog/share open, the per-panel copy modal would re-open empty because this.rendered === true skipped _initUI even though the menu node was gone. Replace the !this.rendered guard in open() with an OR-guard that also fires when .copyMenu has been removed from the DOM. _initUI's inner length === 0 guard keeps it idempotent. The proper root-cause fix (narrowing the cog selector) is deferred: this self-heal is sufficient and the broad selector remains a code smell to triage separately. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Related verses (#385)
The panel options bar carried its own copy icon (.copyDropdownToggle) alongside the navbar #copy-icon. Drop the panel one and keep the navbar button as the single way into the copy menu. The icon could not simply be deleted: step.util.copyModal() — the navbar entry point — worked by locating the active panel's .copyDropdownToggle and dispatching a click on it, so removing the markup would have taken the navbar button down with it. Instead, give the navbar a direct route to the view: - start.jsp drops the <a class="copyDropdownToggle"> icon. The wrapping <span class="dropdown copyDropdown"> stays — it is the positioning anchor for .copyMenu and the holder of the .open state class. - step.copyDropdown gains a views registry (panelId -> view) populated in initialize and cleaned up in remove, so the navbar can reach whichever panel is active. - PassageCopyMenuView.toggle() is split out of onToggleClick as the entry point for callers outside the view. - step.util.copyModal() looks up the active panel's view and calls toggle() instead of clicking an element that no longer exists. onToggleClick, its events entry and the .copyDropdownToggle SCSS are left in place (commented as dormant) so restoring the in-panel icon is a markup-only change. Verified against a local build: panel bar renders no copy icon, navbar button opens/closes the menu, selection and grid modes both copy correctly, the menu follows the active panel across two panels, and the registry is cleaned up when a panel closes. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Comment-only change; no behavioural difference. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The 21 copy_* / copy_dropdown_* keys added in 231fb6e (#375) were hand-written into step-core/src/main/resources/InteractiveBundle.properties. That file is not source-of-truth: the bundles are Crowdin-managed and loaded out-of-band, and Crowdin_script/crowdin/download_bundle.py copies each downloaded file over the tracked one wholesale. Locally-added keys therefore sit in a file that gets bulk-rewritten, so they are a standing merge-conflict surface and can disappear without notice. None of the 101 InteractiveBundle_*.properties variants ever carried them, so every non-English user was already seeing English. Remove all 21 keys; the file is now byte-identical to upstream/master. Each usage site is handled one of three ways: - Reuse a real translated upstream key where one exists: __s.copy and __s.close (HtmlBundle), __s.text_is_copied for the success banner. This is a net i18n gain -- these strings were English-only before, and now render as Copiar / Cerrar / "El texto esta copiado..." under es. - Promote the properties wording into the literal for the remaining 12, so no user-visible string changes. Notably the stale-passage path kept "The passage changed. Re-open the copy menu." rather than falling back to the unrelated "No verses to pick." - Delete outright the 3 that were never referenced: copy_your_selection, copy_button_aria, copy_dropdown_status_ajax_error. Same approach as 6779b86 for the related-verses keys. Verified against a full build: all 21 keys absent from the served __s, the reused keys resolve in en and es, promoted error strings render on their actual code paths, and the existing copy suites pass (22/22 end-to-end, 16/16 phase34; the smoke suite's T8 failure is pre-existing and unrelated). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
#copy-icon carried class="hidden-xs", added in 231fb6e (#375) -- upstream's anchor has no class. That was survivable while the per-panel copy icon existed, since the panel icon had no hidden-xs and served as the phone entry point. Removing the panel icon left nothing: below 768px there was no way to reach copy at all. Measured before this change, #copy-icon was display:none at 375/600/767 and the button is not inside .navbar-collapse, so the hamburger did not reveal it either. Drop hidden-xs from the anchor only. The inner .navbarIconDesc span keeps its hidden-xs, so phones get the glyph without the "Copy" label -- 23px wide on xs vs 58px at >=768. This also makes #copy-icon consistent with every sibling navbar icon (#report-icon, #stats-icon, #bookmark-icon, #fonts-icon, #colorgrammar-icon), all of which have a bare anchor and hide only the label. Note this restores visibility, not tappability on touch phones: header.scss sets #stepnavbar { z-index: auto } under (hover: none) and (max-width: 767px), which lets .mainPanel paint over the whole navbar. That rule is upstream (d09645f, #336) and blocks every navbar control including the hamburger, so it is left alone here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The menu was a Bootstrap dropdown parented to a 0x0 anchor span in the panel
header, so it hung off a single point and landed on top of the passage text
you were trying to copy. It is now draggable by its header, on desktop and on
touch, and remembers where you put it.
Mechanism: dragging switches the menu to position:fixed with z-index 1035.
That is what lets it leave the panel at all — #columnHolder sets
overflow-y:hidden, which clips absolutely-positioned descendants, so an
absolute menu cannot be moved out no matter what left/top say. Nothing in the
ancestor chain sets transform/filter/perspective/contain, so fixed resolves
against the viewport and escapes the clip; 1035 clears #stepnavbar's 1030.
Behaviour:
- first open on desktop: top right of its own panel, under the options bar
- first open at <=640px: unchanged, still the CSS bottom sheet
- the parked position is reused on the next open, shared across panels,
session-scoped and re-clamped on open/resize so it can't strand off-screen
- double-click or double-tap the header, or Home on the grip, to reset
- arrow keys on the grip nudge 10px, shift+arrow 40px
Pointer Events where available, with a mouse+touch fallback for the older iOS
paths this app still carries. Document-level move/end listeners are bound
natively rather than through jQuery so touchmove can be registered
non-passive. touch-action:none and user-select:none on the handle stop the
browser claiming the gesture and stop a drag sweeping a text selection across
the passage underneath.
Two defects found while verifying, fixed here because the drag would
otherwise inherit them:
- The click that terminates a drag lands on whatever is under the pointer.
It both dismissed the menu (it reaches _outsideHandler via the common
ancestor of mousedown/mouseup, so _stopInsideClicks never sees it) and
activated the page beneath — dragging onto a tagged word opened the
lexicon. One click is now swallowed, and only when it lands outside the
dropdown, so a real click on a grid cell right after a drag still works.
- Pre-existing: step_ready.js binds the app's single-key shortcuts on
document *keyup*, while every keyboard handler in this menu works on
keydown, so their stopPropagation() never protected them. Pressing Right
on a verse grid cell moved the cursor and also flipped the panel to the
next chapter, re-rendering the passage and closing the menu. Verified
before (Gen 1 -> Gen 2, menu closed) and after (Gen 1, menu open).
Confined to view_menu_copy.js and copy_dropdown.scss — no JSP change (the
anchor span already exists and the header is built in _initUI), and no vendor
dependency. libs/draggabilly.js was considered and rejected: it loads async
via requirejs, preventDefault()s the whole handle so the close button could
not live in it, and positions in the offsetParent's coordinate space — which
is the 0x0 span we are trying to escape.
Verified against a headless build: new drag suite 37/37 (desktop plus real
CDP touch events), verify_panel_removal 22/22, phase34 16/16, smoke 11/12
with the known pre-existing T8 failure (it reads .copyStatusRow while success
has rendered into .copyFooterSuccess since 493d512; T8b confirms the
clipboard content is correct).
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
copy button update, movable modal
* fix(copy): keep the copy menu out of newly created panels createNewColumn builds a new panel by cloning the active column, and the clone scrub never touched .copyDropdown's children: a panel whose copy menu was open donated a visible orphan to the new panel — the .open class, the injected .copyMenu with its panel-scoped ids, and the inline fixed-position coordinates all came along. No view owned the duplicate, so nothing could close it: it greyed out with its inactive host panel, survived clicks in either panel, and made the next navbar Copy click toggle-close a phantom so the click after that opened in grid mode as though nothing was highlighted. PassageCopyMenuView.initialize now resets its own panel's cloned .copyDropdown span (a fresh menu re-injects on first open) and dismisses a dialog open in any other panel, so opening a new panel always closes the copy menu. Verified with tests/test_copy_multipanel.js in the workspace harness: 13/23 pre-fix (all reported symptoms reproduced), 28/28 post-fix; smoke 12/12, phase34 16/16, wipe-recovery 13/13 unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * feat(copy): pre-select every displayed verse when the dialog opens with no selection Opening the copy dialog with nothing selected used to show an empty verse grid whose footer Copy button looked enabled but silently did nothing (_applyCooldownState blanket re-enabled the button that _updateGridVisuals had just disabled; the click then died on the NaN guard in onPrimaryClick). Now the selection->grid fallback in _update pre-selects the whole displayed range (_gridStart=0.._gridEnd=N-1), so the armed Copy button genuinely copies the chapter in one click. The prefill is skipped for unresolvable snapshots (a real selection we failed to match must not silently become a whole-chapter copy), for non-passage panels (searchType gate - search results would arm hundreds of cross-book hits), and for empty renders. "Pick a different range" still opens an empty grid, a fresh real selection still wins, and the first click on a prefilled grid starts a new range via the existing third-click-reset semantics. Also scope _applyCooldownState's re-enable away from .copyGridPrimary so the grid primary's enablement stays owned by _updateGridVisuals, fixing the enabled-but-inert button on every path. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> * refactor(copy): remove dead code and stale comments from the copy feature copy_text.js: drop _getSelectionState (no callers) and the retired classic-modal probes (#cpyverN, #selectnotes, #selectxref - those ids no longer exist, so the rewrites are falsy-equivalent); replace the unreachable execCommand fallback with a clipboard-denied message (the copy UI is hidden when the async Clipboard API is missing); merge the duplicated verse-trim loops into _removeVerseAncestor; fix an implicit global; drop a write-only errors array; case-correct the .versenumber/.verselink selectors to the camelCase classes the XSL emits (identical under the quirks-mode page, and no longer dependent on it). view_menu_copy.js: remove the never-created .copyDropdownToggle wiring, the _forceClose alias, the dead data-osis cell attribute, the dead footer placeholder and write-only resolution.versions; relocate the unresolvable-selection message from a provably unreachable branch into _update's mode-policy flip so it actually renders; trim stale comments (classic #copyModal rollout, _buildChapterVerseTable). _resolution is kept: the e2e suites read it as an introspection surface. copy_dropdown.scss: delete unused selectors (.copyDropdownToggle, .copyStatus--success/--error/--ajax-error, .copyGridCell--gap and [aria-disabled], .copyPrimaryBtn.disabled); dedupe the success-banner and checkbox styling. view_menu_passage.js: remove a commented-out #copy-icon block. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> --------- Co-authored-by: vercaemert <nvercaemert@gmail.com> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
Removes the "Pick a different range" and "Back to selection" buttons and the whole selection/grid mode dichotomy. Every open now seeds the verse grid from the last passage highlight — matched OSIS-exactly against the displayed verses, with no bare-number fallback, so a selection from a passage no longer displayed misses cleanly into the whole-display prefill instead of silently arming a lookalike range. The recency gate is gone: any still-matching selection prefills regardless of age, and the 60s window now only gates the "couldn't match" warning. Focus lands on the armed Copy chip (else the first grid cell), the seeded range is scrolled into view, and the chip carries a full "Copy X to Y" aria-label. Also erases the latent stuck-disabled back-button quirk after grid-mode copies. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019iMHw7cseNqeqHS3hs6oDE
A new passage highlight now pings step.copyDropdown so an open copy menu re-seeds its grid live. The deselectedAt stamp and its suppression gate protected a recency window the dropdown no longer has — deleted, leaving the collapse branch a plain early return. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019iMHw7cseNqeqHS3hs6oDE
Nothing has rendered these classes since the copy dialog moved to the dropdown menu; the dropdown's own .copySelectionPrimary is gone too. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019iMHw7cseNqeqHS3hs6oDE
Adversarial review of the single-mode collapse confirmed one regression and several rough edges, all fixed here: - _findVerseIndexByOsis now scans every verseLink in the enclosing container and includes tr.row in the ancestor set — column-comparison views keep the OSIS on the per-version cells, so highlights there seeded nothing and showed a spurious "couldn't match" warning. - Esc is routed from anywhere in the menu (the default focus is the armed Copy chip, where Esc previously died); clearing from the chip parks focus on a grid cell; onGridKeydown reads ev.target, fixing grid keyboard navigation through the capture-phase binding. - open() remembers the previously focused element and close() restores it, so keyboard users aren't dropped to <body>. - A highlight made during cooldown is no longer silently ignored — the re-seed renders disabled and arms at cooldown end. - Live re-seeds keep the options strip, so version checkboxes the user just changed aren't reverted by a new highlight. - The seeded-range scroll targets only .copyGridScroller; scrollIntoView also scrolled the overflow-hidden #columnHolder while the menu was still inline, permanently shifting the app layout. - Dead weight: copyPrimaryBtn/copyGridPrimary class split merged, unused active() and _deriveChapterLabel param removed, write-only version/textLength fields dropped from the selection tracker. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019iMHw7cseNqeqHS3hs6oDE
merge fork
A highlight made in one panel was snapshotted into another panel's copy menu, failed to resolve there and, being under 60 s old, left the Copy chip disabled behind the "couldn't match" warning instead of prefilling the whole display. The tracker also re-stamped a still-live highlight whenever a sync-update rewrote the search inputs, because a text control's selectionchange bubbles to the document listener. - tracker records passageId and ignores non-document selectionchange - a fresh unmatched highlight warns only when it came from this panel - the post-copy timer refocuses the chip when focus fell to body - closing a panel drops its highlight (panel ids are recycled) Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EJcAFJ2nCqkMuyHYpo8qak
Copy modal: scope the highlight to its panel
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.