Copy Button Modal Overhaul - #395
Open
nathanvercaemert wants to merge 14 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
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.