feat(image-preview): add zoom support to image lightbox - #965
Conversation
Add yet-another-react-lightbox Zoom plugin to image preview lightbox. Supports: - scroll wheel to zoom in/out - double-click / double-tap to zoom - pinch-to-zoom on touch devices - zoom in button in toolbar maxZoomPixelRatio=4 (4x), zoomInMultiplier=2 per step. Applied to both new UI and legacy UI lightbox instances.
Jerry-Xin
left a comment
There was a problem hiding this comment.
APPROVE ✅ — clean, minimal, correctly wired.
This enables the built-in Zoom plugin of yet-another-react-lightbox (^3.30.0) on both Lightbox instances (new UI + legacy). Byte-verified the +6/-2 delta against head 60f0a099.
Verified
- Scale bounds — safe. No custom zoom math; relies on the library plugin, which clamps zoom to
[1, maxZoomPixelRatio].maxZoomPixelRatio: 4caps max zoom; there is no path to 0/negative/unbounded scale.zoomInMultiplier: 2and the doubleTap/doubleClick delays are standard plugin options. - No listener leak. The PR adds no manual wheel/pinch listeners. The
Zoomplugin registers/tears down its own handlers with the Lightbox lifecycle. Since rendering is gated byopen={showPreview}, closing unmounts and cleans up. - State reset on close. Zoom state is internal to the plugin instance; when
showPreviewflips false the Lightbox unmounts and zoom state is discarded, so it resets on reopen. No stale state carried across opens. - No dead code. All 6 added lines integrate:
Zoomimport +zoom.css,Zoomadded to bothpluginsarrays, and matchingzoom={...}config on both instances. Both instances updated symmetrically. - Accessibility unchanged/improved — plugin supplies toolbar zoom buttons + keyboard support; nothing regressed.
Notes (non-blocking)
- No component tests exist for this cell; change is config-only against a third-party plugin, so no unit-test gap of concern. Manual verification of wheel/pinch/double-tap zoom at runtime is sufficient.
mergeable=MERGEABLE;mergeStateStatus=BLOCKEDis only due to the pendingcode-reviewstatus gate (this review), not a conflict. Other CI checks pass.
LGTM.
lml2468
left a comment
There was a problem hiding this comment.
Review Verdict:需修改(REQUEST_CHANGES)—— 构建挂
审查锚定 head 60f0a099。1 文件 +6/−2,给图片 lightbox 加 yet-another-react-lightbox 的 Zoom 插件。功能方向对、两个 lightbox 实例改动一致、zoom 配置合理,但多引了一行不存在的 CSS 子路径,导致 @octo/web 构建失败。构建挂 = 自动 REQUEST_CHANGES。
🔴 阻断:import "yet-another-react-lightbox/plugins/zoom.css" 不存在 → vite build 失败
Messages/Image/index.tsx:11新增import "yet-another-react-lightbox/plugins/zoom.css";- 但装的版本 3.30.1 的 package
exports没有./plugins/zoom.css这一项(zoom.css: undefined),磁盘上也无该文件 —— v3 里 Zoom 插件的样式已合进主styles.css(第 10 行import ".../styles.css"已引入),不再有独立的plugins/zoom.css; - 实测构建失败(
pnpm --filter @octo/web build),精确报错:[rolldown:vite-resolve] Error: "./plugins/zoom.css" is not exported under the conditions ["module","browser","production","import"] from package yet-another-react-lightbox ✗ Build failed in 1.24s - 修法:删掉第 11 行
import "yet-another-react-lightbox/plugins/zoom.css"即可 —— Zoom 交互样式随主styles.css已加载,不需要单独 import。
✅ 其余(核过,删掉那行 CSS 即全绿)
- Zoom 插件 JS import 正常:
import Zoom from "yet-another-react-lightbox/plugins/zoom"在 3.30.1 下require.resolve命中dist/plugins/zoom/index.js,无问题; - 两个 lightbox 实例改动一致:多图(:282/288)与单图(:368/374)都加了
plugins={[Download, Zoom]}+ 同一套zoom={{ maxZoomPixelRatio:4, zoomInMultiplier:2, doubleTapDelay:300, doubleClickDelay:300 }},配置合理、无遗漏; - 依赖已声明(
dmworkbase/package.json ^3.30.0),无需新增依赖; - 已 rebase(behind 0)、
git merge-tree0 冲突。
结论
功能小而对、两实例一致、zoom 配置合理、Zoom 插件 JS 正常;唯一阻塞是多引了一行 v3 已不存在的 plugins/zoom.css,导致 vite build 失败(样式已在主 styles.css 里)。删掉 Image/index.tsx:11 那行 import 即可构建通过、放行。REQUEST_CHANGES。
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #965 (octo-web)
Summary
The change adds the Zoom plugin to both Lightbox instances (new UI + legacy UI) in packages/dmworkbase/src/Messages/Image/index.tsx. The feature intent is correct and the two instances are updated consistently. However, one of the two new imports references a file that does not exist in the installed library version, which breaks the build. This must be fixed before merge.
1. Spec compliance
Spec: ✅
- Missing: none — zoom is enabled on both Lightbox instances as described.
- Over-build: none — no extra flags/fields/features beyond the stated scope.
- Deviation: none in intent — the change matches the PR description (enable Zoom on the image lightbox).
2. Code quality
Quality: Changes-Requested
🔴 P0 — Build break: plugins/zoom.css subpath does not exist (unresolvable import)
packages/dmworkbase/src/Messages/Image/index.tsx:11
import "yet-another-react-lightbox/plugins/zoom.css"The pinned/resolved version is yet-another-react-lightbox@3.30.1 (packages/dmworkbase/package.json → ^3.30.0; pnpm-lock.yaml resolves 3.30.1). In that version:
- The package
exportsmap defines CSS subpaths only for./styles.css,./plugins/captions.css,./plugins/counter.css, and./plugins/thumbnails.css. There is no./plugins/zoom.cssentry and no wildcard fallback. - No
zoom.css(or any CSS) file ships underdist/plugins/zoom/— the zoom plugin does not have a separate stylesheet in this version.
Because the consuming app (apps/web) bundles with Vite (strict exports resolution) and dmworkbase is consumed as raw source (main: src/index.tsx), this import fails to resolve. Verified directly against the published 3.30.1 package:
require.resolve('yet-another-react-lightbox/plugins/zoom.css')
→ ERR_PACKAGE_PATH_NOT_EXPORTED: subpath './plugins/zoom.css' is not defined by "exports"
Result: vite build (and the dev server) will fail with an unresolved-import error.
The import is also unnecessary: the Zoom plugin in 3.30.1 ships no dedicated CSS, and its zoom-relevant styles are not part of styles.css either (it applies behavior via inline styles). Zoom works with just the plugin JS import + the already-present styles.css.
Fix: remove line 11:
-import "yet-another-react-lightbox/plugins/zoom.css"Keep the import Zoom from "yet-another-react-lightbox/plugins/zoom" (line 9) — that subpath is exported and resolves correctly. After removing the CSS line, please run pnpm --filter @<web-app> build (or the repo's build) to confirm the bundle compiles.
What looks good
plugins={[Download, Zoom]}is applied to both Lightbox instances (new UI ~L282 and legacy UI ~L368), so the two code paths stay consistent andDownloadis preserved.- The
zoom={{ maxZoomPixelRatio, zoomInMultiplier, doubleTapDelay, doubleClickDelay }}config keys are all valid options for the 3.30.x Zoom plugin. import Zoom from "yet-another-react-lightbox/plugins/zoom"(the JS plugin) is a correctly-exported subpath and resolves fine.
3. Verdict
CHANGES_REQUESTED — one P0 build-breaking import (plugins/zoom.css). Removing that single line resolves it; the rest of the change is correct and ready.
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q · automated review]
Verdict: Approve — no blocking findings; notes below (data-flow traced).
Code Review — PR #965 (octo-web)
Reviewer: Octo-Q (automated review)
Summary
This PR adds zoom support to the image lightbox in the image message preview component. It imports the Zoom plugin (and its CSS) from yet-another-react-lightbox, registers it alongside the existing Download plugin on both <Lightbox> instances in Messages/Image/index.tsx, and configures sensible zoom parameters (maxZoomPixelRatio: 4, zoomInMultiplier: 2, doubleTapDelay: 300, doubleClickDelay: 300). The change is small (6 insertions / 2 deletions), well-scoped, and uses an already-pinned dependency (yet-another-react-lightbox@^3.30.0).
Verification
- ✅ Diff scope — single file, +6/-2, only
packages/dmworkbase/src/Messages/Image/index.tsx. - ✅ Dependency present —
yet-another-react-lightbox@^3.30.0inpackages/dmworkbase/package.json:69; the Zoom plugin and its CSS are standard v3 plugin paths. - ✅ Both Lightbox instances updated — the new UI lightbox (
:282) and the legacy/fallback lightbox (:368) both receive[Download, Zoom]and identicalzoom={{...}}config. No parity gap within this file. - ✅ Plugin config values —
maxZoomPixelRatio,zoomInMultiplier,doubleTapDelay,doubleClickDelayare all valid Zoom plugin props per the library API. - ✅ Test mock gap benign —
__tests__/ImageContent.test.tsmocks the lightbox and download plugin but not the zoom plugin. The tests coverImageContent(data class) andgetImageTransferState(state helper), notImageCellrender, so the missing mock is not exercised.
Static analysis only at head 60f0a099; build and tests not executed in this environment.
Findings
No P0/P1 issues. One P2 observation below.
P2 — Zoom not added to 3 sibling Lightbox instances (packages/dmworkbase/src/Messages/Image/index.tsx:282)
The Zoom plugin is correctly wired on both lightbox components in this file, but three other <Lightbox> instances in the codebase still render images without zoom:
Components/Conversation/index.tsx:364— FoldImage (collapsed images in conversation view)Components/MergeforwardMessageList/index.tsx:487— merge-forward message image previewMessages/Text/MarkdownContent.tsx:435— markdown inline image preview
Users can pinch-zoom / scroll-zoom image messages but not markdown images, folded images, or forwarded previews. Not a blocker for this PR, but a follow-up to align the plugin set across all image preview surfaces would prevent the inconsistency from growing.
Data Flow Traceback
Zoomplugin — imported fromyet-another-react-lightbox/plugins/zoom, a well-known library plugin object. Registered in thepluginsarray; the Lightbox component consumes it at render time. No data-flow risk.zoomconfig prop — plain object literal passed as a JSX prop. Values are constants, no upstream data dependency.
Blind-spot Checklist
- C1 (dual path parity) — Within the changed file, both Lightbox instances are updated identically. Three Lightbox instances in other files are not updated (noted as P2 above). These are separate components, not symmetric add/remove pairs, so this is a UX consistency note rather than a parity contract break.
- C2 (control-flow ordering) — N/A. No control-flow changes.
- C3 (auth boundary) — N/A. Pure frontend UI plugin addition.
- C4 (auth lifecycle cascade) — N/A.
- C5 (build/packaging) — The Zoom CSS is imported as a side-effect import. No build config changes needed; the bundler handles CSS imports from
node_modules. No risk. - C6 (governance/docs) — N/A.
Cross-round Blocker Re-check
N/A — first review round.
Verdict: APPROVED
Clean, well-scoped enhancement. The Zoom plugin is correctly added to both Lightbox instances in the image message component with reasonable configuration. No correctness, security, or build concerns. The one P2 (sibling Lightbox instances not updated) is a non-blocking UX consistency note for a follow-up.
[Octo-Q] verdict: APPROVE + 零 P0/P1,仅一个 P2 UX 一致性建议(其他 3 处 Lightbox 未同步加 Zoom),不阻塞合入。
yet-another-react-lightbox v3 merged zoom styles into the main styles.css. The separate plugins/zoom.css path does not exist in the package exports and caused vite build to fail. Remove the redundant import.
lml2468
left a comment
There was a problem hiding this comment.
Review Verdict(复审 @ 635efc7):可合并(APPROVE)—— 从 CR 翻为 APPROVE
复审锚定新 head 635efc7f。上轮的构建阻塞已修:删掉了那行不存在的 plugins/zoom.css import,@octo/web 构建实测通过。
🔴→✅ 上轮阻塞:phantom zoom.css 已删,构建通过
- 上轮
Image/index.tsx多引import "yet-another-react-lightbox/plugins/zoom.css"(v3.30.1 无此 export)→ vite build 失败; - 本 head 该行已删除,只留有效的
import Zoom from "yet-another-react-lightbox/plugins/zoom"(Zoom 交互样式随第 8 行主styles.css已加载); - 实测
pnpm --filter @octo/web build→✓ built in 3.30s(仅剩既有 semi-ui 动态导入 warning,与本 PR 无关)。
✅ 功能核过
- 两个 lightbox 实例一致:多图(:279/285)+ 单图(:365/371)都加
plugins={[Download, Zoom]}+ 同套zoom={{ maxZoomPixelRatio:4, zoomInMultiplier:2, doubleTapDelay:300, doubleClickDelay:300 }}; - 缩放边界安全:直接用插件的 clamp(
[1, maxZoomPixelRatio=4]),无自定义缩放数学、无 0/负/无界路径; - 无监听器泄露:未加手写 wheel/pinch 监听,插件自管理生命周期,
open={showPreview}关闭即卸载;缩放态在插件实例内,重开归零; - 依赖已声明(
^3.30.0,实装 3.30.1)、已 rebase(behind 0)、0 冲突。
🟡 非阻塞(mochashanyao)
- 仓库其它 Lightbox 实例(本 cell 之外)未加 Zoom,存在体验不一致 —— 属后续统一,非本 PR 范围,不阻断。
结论
上轮的 phantom zoom.css 构建阻塞已删净、build 实测通过;两实例 zoom 配置一致、缩放边界安全、无监听器泄露。无阻塞。从 REQUEST_CHANGES 翻为 APPROVE。
yujiawei
left a comment
There was a problem hiding this comment.
Code Review — PR #965 (octo-web)
Reviewed against head 635efc7 (merge base main @ 8246e067). This is a config-only change: enable the built-in Zoom plugin of yet-another-react-lightbox (3.30.1) on both Lightbox instances in packages/dmworkbase/src/Messages/Image/index.tsx. Net diff on head is +5/−2.
1. Spec compliance
Spec: ✅
- Missing: none — the stated goal (enable zoom in the image lightbox) is fully implemented.
Zoomis imported and added to thepluginsarray on both instances: the new-UI Lightbox (index.tsx:281) and the legacy-UI Lightbox (index.tsx:367), each with a matchingzoom={...}config. - Over-build: none — no extra flags, fields, or behavior beyond what the PR describes.
- Deviation: none — implementation matches the described approach exactly (library plugin, no custom zoom math).
2. Code quality
Quality: Approved
Verified:
- Import resolves.
yet-another-react-lightbox/plugins/zoommaps to./dist/plugins/zoom/index.jsin the installed 3.30.1exportsmap — the import is valid. - Build break fixed. The earlier
import ".../plugins/zoom.css"(which broke the vite build because that subpath is not exported and does not exist on disk in v3 — the plugin styles are bundled into the already-importedstyles.css) was removed in commit635efc7. Confirmed./plugins/zoom.cssisundefinedin the packageexportsand absent on disk, and the current head no longer imports it. - Valid config. All four options —
maxZoomPixelRatio,zoomInMultiplier,doubleTapDelay,doubleClickDelay— are declared in the plugin's type definitions.maxZoomPixelRatio: 4bounds max zoom; there is no path to zero/negative/unbounded scale. - No listener leak. No manual wheel/pinch handlers added. The plugin registers/tears down its own handlers with the Lightbox lifecycle, and rendering is gated by
open={showPreview}, so closing unmounts and cleans up. Zoom state is internal to the plugin instance and resets on reopen. - Symmetric. Both Lightbox instances are updated identically, so behavior is consistent across the new and legacy UIs.
No P0/P1/P2 findings.
Notes (non-blocking)
- No component tests exist for this cell. Since the change is config-only against a third-party plugin, a unit-test gap is not a concern here; runtime verification of wheel/pinch/double-tap is sufficient.
3. Overall verdict
APPROVE — Spec ✅ and Quality Approved. The prior CHANGES_REQUESTED was against the earlier SHA 60f0a099 for the non-existent zoom.css import; that build break is resolved on the current head.
mochashanyao
left a comment
There was a problem hiding this comment.
[Octo-Q · automated review]
Verdict: Approve — no blocking findings; notes below (data-flow traced).
Code Review — PR #965 (octo-web)
Reviewer: Octo-Q (automated review)
Head SHA: 635efc7f9b179ec38706f5c787d04d0fc69ce2a4
Merge-base: 8246e067c956fa007ea90959e7b14e734a52b7c3
Summary
This PR adds zoom support to the image lightbox in the IM message view. It imports the Zoom plugin from yet-another-react-lightbox/plugins/zoom and registers it alongside the existing Download plugin on both Lightbox instances (new UI at packages/dmworkbase/src/Messages/Image/index.tsx:281 and legacy UI at :367). A second commit removes a stray import for a non-existent zoom.css file that was erroneously added in the first commit. The change is minimal (5 insertions, 2 deletions, 1 file), well-scoped, and consistent across both render paths.
Verification
- ✅ Dependency exists —
yet-another-react-lightbox@3.30.1resolved inpnpm-lock.yaml; theplugins/zoommodule is a documented plugin in v3.x. - ✅ Import path correct —
yet-another-react-lightbox/plugins/zoomis the canonical import for the Zoom plugin. - ✅ Config options valid —
maxZoomPixelRatio,zoomInMultiplier,doubleTapDelay,doubleClickDelayare all documented Zoom plugin options in v3.x. - ✅ Dual-path parity — both Lightbox instances (new UI
:281/:287and legacy UI:367/:373) receive identical plugin registration and zoom configuration. C1 satisfied. - ✅ No data-flow regression — the PR only adds props to existing
<Lightbox>components; no existing data flow (slides, download handler, carousel, controller) is modified.
Static analysis only at head 635efc7f; build and tests not executed in this environment.
Findings
No P0/P1 issues. No correctness, security, build, or functional concerns.
Data Flow Tracing
Zoomplugin — consumed atplugins={[Download, Zoom]}in both Lightbox instances. Source:import Zoom from "yet-another-react-lightbox/plugins/zoom". Data flows correctly: the plugin is instantiated by the Lightbox component and receives its config from thezoomprop.zoomconfig object — consumed aszoom={{ maxZoomPixelRatio: 4, zoomInMultiplier: 2, doubleTapDelay: 300, doubleClickDelay: 300 }}in both instances. These are static literal values; no upstream data source to trace.slidesarray — unchanged by this PR. New UI: derived fromuiProps.imagesoruiProps.singleImage?.src. Legacy UI: derived fromimageURLviagetImageSrc(content). Both paths remain intact.
Blind Spot Checklist
- C1 — Dual-path parity: ✅ HIT — verified. Both Lightbox instances (new UI and legacy) have identical
pluginsandzoomprops. No asymmetric configuration. - C2 — Control-flow ordering: N/A — no control-flow changes; the Zoom plugin is purely additive and does not interact with existing plugin ordering semantics.
- C3 — Authorization boundary: N/A — no auth/permission changes.
- C4 — Authorization lifecycle: N/A — no auth changes.
- C5 — Build/runtime path: N/A — no build config, packaging, or environment changes. Plugin is a runtime import resolved by the bundler from a declared dependency.
- C6 — Governance/docs: N/A — no docs or policy changes.
Cross-round Blocker Recheck
N/A — first review of this PR.
Verdict: APPROVED
Clean, well-scoped feature addition. The Zoom plugin is correctly imported and configured identically on both Lightbox render paths. No data-flow regressions, no security concerns, no dual-path asymmetry. Ship it.
[Octo-Q] verdict: APPROVE — no P0/P1 findings; zoom plugin correctly wired with identical config on both Lightbox instances; dependency resolved at v3.30.1.
Jerry-Xin
left a comment
There was a problem hiding this comment.
✅ APPROVE — build break fixed, verified by building
Re-review at head 635efc7f. This supersedes my earlier APPROVE at 60f0a099, which was an under-call: I approved without running the actual build and missed a hard build break. Full credit to @lml2468, Octo-PR, and @yujiawei for catching it — they were right and I was wrong.
Root cause (prior head 60f0a09)
packages/dmworkbase/src/Messages/Image/index.tsx imported yet-another-react-lightbox/plugins/zoom.css, but the installed version is 3.30.1, whose exports map has no ./plugins/zoom.css entry (the Zoom plugin styles ship inside the main styles.css, already imported one line above). Vite therefore failed with "./plugins/zoom.css" is not exported ....
What changed 60f0a09 → 635efc7
- Compare is clean:
ahead 1, behind 0(no rebase drift). Single file touched, 1 deletion, 0 additions. - The deletion is exactly the phantom line:
- import "yet-another-react-lightbox/plugins/zoom.css". - Retained and correct:
import "yet-another-react-lightbox/styles.css"(this bundles the Zoom plugin CSS in 3.30.1).import Zoom from "yet-another-react-lightbox/plugins/zoom"(JS import resolves fine).- Both Lightbox instances (new-UI + legacy) keep
plugins={[Download, Zoom]}and identicalzoom={{ maxZoomPixelRatio: 4, zoomInMultiplier: 2, doubleTapDelay: 300, doubleClickDelay: 300 }}— consistent, sane bound.
- No leftover phantom import, no dead code.
Authoritative check — the build now passes
Checked out 635efc7f, pnpm install --frozen-lockfile, then pnpm --filter @octo/web build:
- Result:
✓ built, exit 0 (reproduced twice, ~2.6–3.4s). No"not exported"/ zoom.css error. - Remaining console output is pre-existing and unrelated (lottie/pdfjs
evalnotices, chunk-size warnings).
Tests
packages/dmworkbase/src/Messages/Image/__tests__/ImageContent.test.ts: 14/14 pass.- The broader
@octo/basesuite shows 72 failures, but they are all pre-existing environment noise (localStorage.clear()→localstorage-file not providedin WKLayout/etc.), none in the Image path and unrelated to this one-line change.
Status
mergeable: MERGEABLE(mergeStateBLOCKED= awaiting required approval/checks).
Zoom feature wired correctly on both lightbox instances, build green, no blockers. LGTM. 修复精准、构建实测通过,批准合入。
问题
点开消息中的图片,无法缩放。
原因
Lightbox (
yet-another-react-lightbox) 已集成在项目中,但plugins数组里只有Download,缺少Zoomplugin。改动
packages/dmworkbase/src/Messages/Image/index.tsx:Zoomplugin 及其 CSS(yet-another-react-lightbox/plugins/zoom)Zoomplugin缩放行为
最大放大倍数:4x,每步 2x。