Conversation
📝 WalkthroughWalkthroughA preprocessor-style conditional was added around the DOCX render-mode branch in a content selector, enclosing the existing docx-handling logic with app-only markers so that this code path is excluded from non-app builds. No functional logic was changed. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
ui/redux/selectors/content.js (1)
296-298:⚠️ Potential issue | 🔴 CriticalAdd missing
//@ifTARGET='app'guard around the docx check inselectFileRenderModeForUri.The docx check in
selectFileRenderModeForUri(lines 296–298) lacks the//@ifTARGET='app'guard that exists in the equivalentmakeSelectFileRenderModeForUriselector. WhileselectFileRenderModeForUriis not directly used in the codebase, it powersselectIsPlayableForUriwhich is called from content.js actions that execute on all builds, allowing unguarded DOCX rendering on web and triggering the crash this PR aims to fix.Proposed fix
if (['text', 'document', 'script'].includes(mediaType)) { return RENDER_MODES.DOCUMENT; } + // `@if` TARGET='app' if (extension === 'docx') { return RENDER_MODES.DOCX; } + // `@endif`
287099c to
e90905c
Compare
|
Coderabbit thing is now handled |
This was crashing, due to DocxViewer only being imported on the desktop App.
But importing it on web doesn't seem to work.
This change defaults .docx to render in the Download only view
https://odysee.com/@Dimitar:6/On-raw-Milk:6
Summary by CodeRabbit