Repository navigation
feat(composer): choose Enter or Cmd/Ctrl+Enter to send messages - #2431
zandt-square wants to merge 3 commits into
Conversation
Co-authored-by: Fizz <fbf1920f313ab37ffa4c517e2437e247fe1423378e077eccc4e29f15625b9918@buzz.block.builderlab.xyz> Signed-off-by: Zandt <zandt@squareup.com>
Wait for Base UI to expose the option instead of synchronously querying its asynchronous popup. Co-authored-by: Fizz <fbf1920f313ab37ffa4c517e2437e247fe1423378e077eccc4e29f15625b9918@buzz.block.builderlab.xyz> Signed-off-by: Zandt <zandt@squareup.com>
The fixture previously inherited the owner pubkey, which fromOwner intentionally accepts. Preserve rejection coverage with an explicit non-owner signer; do not change production admission policy. Co-authored-by: Fizz <fbf1920f313ab37ffa4c517e2437e247fe1423378e077eccc4e29f15625b9918@buzz.block.builderlab.xyz> Signed-off-by: Zandt <zandt@squareup.com>
6c444ce to
0489997
Compare
kazemek21
left a comment
There was a problem hiding this comment.
🤖 Kyle reviewed this PR and the Nexus review-tracker notes and approved with no comments.
Posted by Codex (gpt-6.1-sol) on behalf of kylez.
|
Fizz (AI agent): Rebased locally onto main at e121e52 and dropped the redundant harness-fixture correction now that #2444 supplies the upstream fix. The local branch is one squashed composer-only commit, d600ed5; the harness test file matches main exactly, and the composer implementation patch is unchanged. Publication is blocked: the full local Vitest run and subsequent pre-push related-test gate stalled/exceeded execution limits; Chromium passed both new journeys, but WebKit did not complete. I have not bypassed hooks or force-pushed an unchecked result. The remote PR still points to 0489997. Stopped my own stalled push process tree. Local worktree is clean at ~/.buzz/REPOS/buzz-app-send-keys; the next step is diagnosing the stalled local test gate, then a lease-protected push. Buzz replies are also currently refused with “not a channel member,” so this comment records the handoff. |
|
🤖 Correction to my design review at People can already choose Settings → Shortcuts → Sending messages → “Send with ⌘ Enter” (Ctrl+Enter elsewhere). Enabling it makes Enter add a line and the modified Enter send. Current Settings owner; composer behavior. This PR adds the same choice under Appearance with a separate stored preference. Recommend closing it as a duplicate, as bmo already reported. This replaces my earlier launch-policy assessment. Source-only comparison against main snapshot |
|
🤖 This duplicates the existing send-key preference added in #2331, merged November 5. It is already available under Settings → Shortcuts → Sending messages → Send with ⌘ Enter (Ctrl+Enter on Windows/Linux). Enabling it makes Enter insert a newline and ⌘/Ctrl+Enter send. The existing implementation lives in Recommend closing this PR as a duplicate of #2331. The earlier launch-policy assessment should be corrected: this capability is already in core, so treating it as a new setting that must move to an external plugin does not reflect the current implementation. |
Summary
Uses the existing shared composer and SettingsGroup/PreferenceRow/Select/ToastNotice/Button owners. No new CSS, plugin API, signing policy, or relay protocol changes. Local Block development sign-in configuration is git-ignored and is not included.
Validation
Implementation head:
f0afac6e3cc5a27a5c6c35c03584dbfbd0e91c8f, rebased onto upstream73255c5d7bb1cfa7c454a2d33335053c2ac18165.composer-send-key.spec.mjsandsend-key-settings.spec.mjs. Covers native editor line splitting, send, actual preference persistence and draft restoration after reload, host Settings updating a retained composer, both themes and a narrow 480px layout at 200% interface size. Fixtures use ephemeral keys and the browser host; these do not prove native identity/relay acceptance.SidebarNavigation.test.tsxcompiled-style test hitting its 5-second timeout. Its cause/baseline status is not yet established; no claim of a green full suite.MessageComposer500 tests andAppearanceSettings23 tests passed, including the new modifier/IME/repeat/list/thread/default-switching and persistence/recovery coverage.Scope and remaining checks
Two browser scenarios added, no scenarios removed. Real browser/native caret handling and Settings-to-retained-editor integration justify this layer; modifier, IME, repeat and recovery matrices stay in mounted Vitest tests.
Draft: inspect the current hosted CI run before ready-for-review. The earlier local timeout passed on the later full-suite run described below. Broader Node/Rust/browser suites and post-rebase native acceptance remain deferred. No merge requested.
CI correction —
0277bf44bc16ae9e8498dd998f9e526039a46790The initial hosted run's only failing job was JavaScript shard 3: the Windows preference test synchronously queried an option before Base UI exposed its popup. Changed
getByRoleto awaitedfindByRole, matching neighboring Select tests. No retry, longer timeout, weaker assertion, or product-code change.f0afac6working tree before committing: 16,449 passed, 21 skipped (913 files passed, 7 skipped), 875.91s with two workers. The earlier SidebarNavigation timeout passed in this run; no claim that every potential timing failure is eliminated.f0afac6is historical, not a result for the corrected commit.Separate upstream fixture correction —
6c444ceThe second hosted failure is also present on upstream main: https://github.com/block/buzz-app/actions/runs/41865346731. New
harnessKitrejection coverage inherited a direct-owner signer for its workflow-shaped message, while the current deliberatefromOwnercontract accepts direct-owner messages (see #2399). Explicitly use the existing untrustedmallorysigner in this rejected fixture. Preserve the rejection assertion and leave all production authorization code unchanged. The requester explicitly approved including this as a separate commit.a457243via a merge (no published history rewritten).6c444ce.Linear-history correction
At the requester's direction, removed the main merge commit and rebased the three authored commits onto upstream
1da5031. Published with an explicit lease for prior remote6c444ce; no unrelated remote work overwritten. Current commits:c403563: composer setting1de44dd: asynchronous Select option synchronization0489997: separately approved untrusted-signer fixture correctionAdded/removed patch lines against the corresponding bases are identical before and after rebase. Every commit retains DCO sign-off and co-author credit. Confirmed zero merge commits in the PR and remote head equals local. Current-head push hooks passed; both composer browser journeys passed again in Chromium and WebKit (4/4). Full-suite 16,533-pass evidence above is pre-rebase; broad checks on the new upstream base remain with hosted CI. The prior merge-based run is superseded.