Skip to content

fix popup cmd a select all for #1395#1396

Open
Chapoly1305 wants to merge 2 commits intowindingwind:mainfrom
Chapoly1305:codex/fix/popup-cmd-a-select-all
Open

fix popup cmd a select all for #1395#1396
Chapoly1305 wants to merge 2 commits intowindingwind:mainfrom
Chapoly1305:codex/fix/popup-cmd-a-select-all

Conversation

@Chapoly1305
Copy link
Contributor

@Chapoly1305 Chapoly1305 commented Mar 5, 2026

Root cause was missing event wiring: the popup textarea’s keydown handler (getOnTextAreaCopy) was no longer attached after refactor, so Cmd/Ctrl+A logic in that handler never ran. metaKey detection itself was fine; it just wasn’t being invoked in the popup textarea. getOnTextAreaCopy(...) centralizes Cmd/Ctrl+A/C/X behavior in one place.
It avoids duplicating shortcut logic across multiple inline listeners.

Test results: now user able to use select all in the pop-up, as well as copy, cut, etc. Concat feature works as before, not affected by this fix.

image

Copilot AI review requested due to automatic review settings March 5, 2026 02:48
@Chapoly1305 Chapoly1305 mentioned this pull request Mar 5, 2026
1 task
Copy link

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes missing popup textarea keyboard shortcut handling so Cmd/Ctrl+A/C/X work again in the reader selection popup (regression from refactor), while also making concat-modifier tracking reflect the actual modifier-pressed state.

Changes:

  • Re-attaches the popup textarea keydown handler via getOnTextAreaCopy(...) so select-all/copy/cut shortcuts run.
  • Refactors concat-modifier tracking to use ev.metaKey/ev.ctrlKey state rather than only reacting to modifier keydown.
  • Minor inline handler tweak in standalone.xhtml (window.close()).

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/modules/shortcuts.ts Tracks concat modifier using metaKey/ctrlKey state on keydown/keyup.
src/modules/popup.ts Wires keydown listener for popup textarea to restore Cmd/Ctrl+A/C/X behavior.
addon/chrome/content/standalone.xhtml Small inline handler change for close command.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

@Chapoly1305
I automatically applied lint fixes but couldn’t push to your branch—could you please run:

npm run lint

and push the results? Thanks!

@Chapoly1305 Chapoly1305 force-pushed the codex/fix/popup-cmd-a-select-all branch from 556e444 to d2b5c22 Compare March 7, 2026 19:00
@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

@Chapoly1305
I automatically applied lint fixes but couldn’t push to your branch—could you please run:

npm run lint

and push the results? Thanks!

@github-actions
Copy link
Contributor

github-actions bot commented Mar 7, 2026

@Chapoly1305
I automatically applied lint fixes but couldn’t push to your branch—could you please run:

npm run lint

and push the results? Thanks!

@Chapoly1305 Chapoly1305 marked this pull request as draft March 7, 2026 19:03
@Chapoly1305 Chapoly1305 marked this pull request as ready for review March 7, 2026 19:07
@Chapoly1305
Copy link
Contributor Author

Test passed on MacOS,

  1. Cmd+A/C/X works on pop-up
  2. Concat works was expected.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants