sync: pull upstream dev (38b64b3ef) - #439
Conversation
Code Coverage OverviewLanguages: JavaScript JavaScript / code-coverage/vitestThe overall coverage in the Show a code coverage summary of the most impacted files.
Updated |
…ync/upstream-dev-into-integration
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description Fixes mentions not sending in non-textual replies, idk what else there is to say about this <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes SableClient#999 #### Type of change - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> Written w the assistance of having forgotten to write this part a few months ago (cherry picked from commit 1499b5d)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> ### Description Adds a context menu for copying and downloading images in the `ImageViewer`. Useful for electron/tauri users. <img width="1036" height="1086" alt="image" src="https://github.com/user-attachments/assets/e3b07381-c76c-4bbe-8b5c-e9492bb7d10c" /> #### Type of change - [ ] Bug fix (non-breaking change which fixes an issue) - [x] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update ### Checklist: - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [x] I have made corresponding changes to the documentation - [x] My changes generate no new warnings ### AI disclosure: - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). Fueled by the rage of using sable electron. (cherry picked from commit 38b64b3)
<!-- Please read https://github.com/SableClient/Sable/blob/dev/CONTRIBUTING.md before submitting your pull request --> <!-- Please include a summary of the change. Please also include relevant motivation and context. List any dependencies that are required for this change. --> Fixes #556 and also fixes the cancel button in the login page having the exact same behavior as the add account - [x] Bug fix (non-breaking change which fixes an issue) - [ ] New feature (non-breaking change which adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to not work as expected) - [ ] This change requires a documentation update - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [ ] Partially AI assisted (clarify which code was AI assisted and briefly explain what it does). - [ ] Fully AI generated (explain what all the generated code does in moderate detail). <!-- Write any explanation required here, but do not generate the explanation using AI!! You must prove you understand what the code in this PR does. --> (cherry picked from commit 7c931bf)
Sentry Preview Error TriageNo Sentry errors found for this PR's preview deployment as of Sun, 28 Jun 2026 13:42:14 GMT. This comment updates automatically after each push. |
…dev-into-integration
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f94179d8d3
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Deploying with
|
| Status | Preview URL | Commit | Alias | Updated (UTC) |
|---|---|---|---|---|
| ✅ Deployment successful! | https://pr-439-charm.justin-tech.workers.dev | 9763020 | pr-439 |
Sun, 28 Jun 2026 13:44:11 GMT |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 48ff174c4f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
|
|
||
| return getMentionContent( | ||
| [replyDraft.userId], | ||
| roomMention || replyEvent?.getContent()['m.mentions']?.room === true |
There was a problem hiding this comment.
Don't inherit @room mentions from replies
When a user replies to a message that already had m.mentions.room === true, this copies that flag into the new event even if the new text/upload/sticker does not mention @room. Since Matrix room-mention notifications are driven by the new event's content.m.mentions.room, any non-silent reply from a user with room-notify power will ping the whole room unexpectedly; only the current event's explicit roomMention should set this flag.
Useful? React with 👍 / 👎.
Description
Selective sync of upstream
dev(SableClient/Sable, upstream HEAD38b64b3ef) intointegration.Rather than a single all-or-nothing merge, the 9 upstream PRs since the divergence point (
#943) were evaluated individually. Each was tested as an isolated cherry-pick ontointegrationand typechecked. Most of the original conflict noise came from two large upstream PRs (#946,#986) in areas Charm has already reworked independently — those are intentionally not merged here (see "Deferred" below).Pulled in (this branch)
usePathWithOrigin, preserves Charm'sstopClient().finally()pattern)tsc --noEmitandoxlintpass clean on the result.Already present in
integration(no-op — Charm implemented the equivalent independently)Overlay/FocusTrapdialog.if (question).MessageCopyTextItem(handles edited/redacted messages, uses Charm's icon set). Upstream's version would be a duplicate declaration.isAddingAccount/loginUrl/registerUrl. (Cherry-pick was dropped after it produced duplicate-declarationtscerrors.)Deferred — investigated in depth, nothing to port
Both large PRs were analyzed file-by-file against Charm's current tree. Conclusion: neither contains a fix Charm lacks.
#946 "Feat better mobile context menu" — Charm is a strict superset
Charm's
ade0bcb2dimported SableClient#946 byte-for-byte (git diff 0b430d860 ade0bcb2donMessage.tsxis empty), then stacked 11 follow-up fix commits on the same surface — includingfix(options): correct sticker type typo and stale Date ref,stop duplicate mobile context menus,clear long-press timer on touchcancel, and the PR #410 review series. Upstream froze SableClient#946 at merge (only later touch was SableClient#1001's copy button, already superseded). Every SableClient#946 piece —MobileOptionsmodal type, optionalSwipeableMessageWrapper.onReply,fix_ghost_quickmenu, arbitrary-event reactions — is already in Charm. The ~342/-144 line divergence on these files is almost entirely Charm's own additions. Nothing to port.#986 "Fix the new sidebar items" — rejected design; micro-fixes already present
oldSidebartoggle + new-sidebar restructure (SidebarNav,UserQuickTools,SettingsTab,settings.ts,Themes.tsx) — iteration on the sidebar Charm forked away from. Charm deletedSettingsTab.tsxandUserQuickTools.*and built its own. Porting would resurrect intentionally-removed files. Not applicable.RenderBody.tsxPopOutreorder (present), tab icon sizingsize 300→400+inline → isBottom ? 'inline' : 'toolbar'(present),SettingsTab"Search"→"Settings"tooltip typo (moot — file deleted in Charm).Sidebar.css.tsPUSH_Y 0→3) is trivial cosmetic tied to upstream's layout.Recommendation: SableClient#946 and SableClient#986 can be considered resolved against this sync — no follow-up port required.
Type of change