Add media-only downloads to drop mobile menu#2341
Open
Conversation
Signed-off-by: prxt6529 <prxt@6529.io>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughMoves media download from a centralized "More" menu into per-media action rows: adds download helpers, per-media UI, and viewer toolbar simplifications; removes the centralized download component and docs; updates video/image sizing and props; adjusts dropdown portal z-index; and expands/updates tests for the new behavior. ChangesMedia download redistribution (single cohesive DAG)
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Viewer as Media Viewer
participant Helper as Media Download Helper
participant Native as Native Share / Browser
User->>Viewer: reveal download UI (hover/click) / click "Download"
Viewer->>Helper: downloadMediaUrl({ url, fileName, isCapacitor, dialogTitle })
alt isCapacitor
Helper->>Native: Share.share(blob, { dialogTitle })
Native-->>Helper: share result
else browser
Helper->>Native: createObjectURL(blob) & trigger anchor download
Native-->>Helper: revokeObjectURL
end
Helper-->>Viewer: resolve / throw
Viewer-->>User: update UI (isDownloading / fallback)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
Signed-off-by: prxt6529 <prxt@6529.io>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx`:
- Around line 127-139: The download button is only visually hidden but remains
in the accessibility/tab order; update DropListItemContentMediaVideo to remove
it from the DOM or make it unfocusable when showDownloadButton is false: either
render the <button> only when showDownloadButton is true, or keep it rendered
but add attributes when hidden (e.g., aria-hidden="true", tabIndex={-1} and
remove it from pointer/events) so it is not reachable by keyboard/screen
readers; apply the change around the element that uses showDownloadButton and
ensure showDownloadButtonTemporarily and handleDownload continue to work when
the button is shown.
In `@components/drops/view/item/content/media/MediaDisplayVideo.tsx`:
- Around line 142-156: The download button is only visually hidden via
opacity/pointer-events which keeps it in the accessibility tree; update the
MediaDisplayVideo.tsx button rendering so that when showDownloadButton is false
it is removed from keyboard/AT focus—either render the button conditionally only
when showDownloadButton is true (inside the existing showControls block) or, if
you must keep it rendered for layout, set aria-hidden="true" and tabIndex={-1}
when showDownloadButton is false (and remove those attributes when true); keep
the existing onClick={handleDownload} and
onFocus={showDownloadButtonTemporarily} behavior when visible.
In `@helpers/media-download.helpers.ts`:
- Around line 6-8: The catch branch that extracts a filename from url currently
returns percent-encoded names for relative URLs; update the logic where name is
computed (using url.split("?")[0]?.split("#")[0]?.split("/").pop()) to run
decodeURIComponent on the extracted name before returning and guard
decodeURIComponent in a try/catch to return fallback if decoding throws (i.e.,
compute const rawName = ...; let name = rawName ? decodeURIComponent(rawName) :
undefined; return name || fallback). Ensure you reference the same url and
fallback variables and keep the existing splitting behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b476d0c6-adce-422a-8ee9-bb8d6357765e
📒 Files selected for processing (20)
__tests__/components/DropListItemContentMediaImage.test.tsx__tests__/components/drops/view/item/content/media/DropListItemContentMediaVideo.test.tsx__tests__/components/drops/view/item/content/media/MediaDisplayVideo.test.tsx__tests__/components/waves/drops/WaveDropActionsDownload.test.tsx__tests__/components/waves/drops/WaveDropActionsMore.test.tsx__tests__/components/waves/drops/WaveDropMobileMenu.test.tsx__tests__/components/waves/drops/WaveDropPartContentFullWidthImage.test.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaImage.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaVideo.tsxcomponents/drops/view/item/content/media/MediaDisplayVideo.tsxcomponents/waves/drops/WaveDropActionsDownload.tsxcomponents/waves/drops/WaveDropActionsMore.tsxcomponents/waves/drops/WaveDropMobileMenu.tsxcomponents/waves/drops/WaveDropPartContentFullWidthImage.tsxdocs/api-tool/feature-api-authentication-and-media-drop-flow.mddocs/waves/drop-actions/README.mddocs/waves/drop-actions/feature-image-viewer-and-scaling.mddocs/waves/drop-actions/feature-media-download.mdhelpers/capacitorBlobDownload.helpers.tshelpers/media-download.helpers.ts
💤 Files with no reviewable changes (5)
- tests/components/waves/drops/WaveDropActionsDownload.test.tsx
- components/waves/drops/WaveDropMobileMenu.tsx
- docs/waves/drop-actions/feature-media-download.md
- docs/api-tool/feature-api-authentication-and-media-drop-flow.md
- components/waves/drops/WaveDropActionsDownload.tsx
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/drops/view/item/content/media/DropListItemContentMediaImage.tsx (1)
147-157:⚠️ Potential issue | 🟠 Major | ⚡ Quick winRemove the button semantics or wire this region back to a real action.
This container is still exposed as a focusable
role="button", but after removing the view-details handler it no longer does anything on Enter/Space. That leaves keyboard and assistive-tech users on a dead control.Suggested fix
- <div - role="button" - className="tw-flex tw-min-h-0 tw-min-w-0 tw-flex-1 tw-flex-col tw-items-center tw-justify-center" - onClick={(e) => e.stopPropagation()} - tabIndex={0} - aria-label="Full size drop media" - onKeyDown={(e) => { - if (e.key === "Enter" || e.key === " ") { - e.stopPropagation(); - } - }} - > + <div + className="tw-flex tw-min-h-0 tw-min-w-0 tw-flex-1 tw-flex-col tw-items-center tw-justify-center" + onClick={(e) => e.stopPropagation()} + >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/DropListItemContentMediaImage.tsx` around lines 147 - 157, The div in DropListItemContentMediaImage currently has role="button", tabIndex={0}, aria-label and key handlers but no action—remove the button semantics or restore an actionable handler: either delete role="button", tabIndex and aria-label (and the onKeyDown/onClick stopPropagation) so it is not focusable, or wire it back to a real handler (e.g., call the existing view-details function or a provided onOpenFullSize handler) and ensure onClick and onKeyDown invoke that action (Enter/Space) instead of only stopPropagation; update the element accordingly in DropListItemContentMediaImage to keep keyboard/AT behavior correct.
♻️ Duplicate comments (3)
components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx (1)
148-162:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the collapsed download button out of the accessibility tree.
tw-opacity-0plustw-pointer-events-noneonly hides this visually. The button still stays in the tab order and accessibility tree while hidden, so keyboard and screen-reader users can reach a control that is supposed to be collapsed. This is the same issue flagged previously.Suggested fix
<button aria-label="Download video" + aria-hidden={!showDownloadButton} + disabled={!showDownloadButton} + tabIndex={showDownloadButton ? 0 : -1} className={`tw-absolute tw-right-3 tw-top-[calc(50%-1.75rem)] tw-z-10 tw-flex tw-size-9 -tw-translate-y-1/2 tw-items-center tw-justify-center tw-rounded-full tw-border-0 tw-bg-black/65 tw-p-1.5 tw-text-white tw-shadow-lg tw-backdrop-blur-sm tw-transition-opacity tw-duration-200 desktop-hover:hover:tw-bg-black/80 ${ showDownloadButton ? "tw-pointer-events-auto tw-opacity-100" : "tw-pointer-events-none tw-opacity-0" }`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx` around lines 148 - 162, The download button is only visually hidden with tw-opacity-0 and tw-pointer-events-none but remains in the accessibility tree; update the button rendering in DropListItemContentMediaVideo so when showDownloadButton is false it is removed from keyboard and assistive technology access (e.g., add aria-hidden="true" and tabIndex={-1} or render null) and when true ensure aria-hidden is false and tabIndex is not -1 so handleDownload, onFocus/showDownloadButtonTemporarily and the ArrowDownTrayIcon are only reachable when visible; also prevent onFocus logic from running when hidden.helpers/media-download.helpers.ts (1)
8-10:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winDecode the fallback filename before returning it.
The
catchpath still returns percent-encoded names for relative URLs, so downloads like/media/image%20name.pngwill surface asimage%20name.png. This is the same bug flagged in the previous review.Suggested fix
- const name = url.split("?")[0]?.split("#")[0]?.split("/").pop(); - return name || fallback; + const name = url + .split("?")[0] + ?.split("#")[0] + ?.split("/") + .filter(Boolean) + .pop(); + if (!name) { + return fallback; + } + try { + return decodeURIComponent(name); + } catch { + return name; + }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@helpers/media-download.helpers.ts` around lines 8 - 10, The catch block in helpers/media-download.helpers.ts returns percent-encoded filenames (variable name) and fallback as-is; change it to decode the filename before returning by running decodeURIComponent on the extracted name and on fallback (safely: wrap decodeURIComponent in a try/catch to fall back to the original value if decoding throws), and return the decoded name || decoded fallback instead of the raw values.components/drops/view/item/content/media/MediaDisplayVideo.tsx (1)
163-179:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winKeep the collapsed download button out of the accessibility tree.
opacityandpointer-eventsonly hide the control visually. WhenshowDownloadButtonis false, it can still be tabbed to or announced by screen readers. This is the same accessibility issue flagged previously.Suggested fix
{showControls && ( <button aria-label="Download video" + aria-hidden={!showDownloadButton} + disabled={!showDownloadButton} + tabIndex={showDownloadButton ? 0 : -1} className={`tw-absolute tw-right-3 tw-top-[calc(50%-1.75rem)] tw-z-10 tw-flex tw-size-9 -tw-translate-y-1/2 tw-items-center tw-justify-center tw-rounded-full tw-border-0 tw-bg-black/65 tw-p-1.5 tw-text-white tw-shadow-lg tw-backdrop-blur-sm tw-transition-opacity tw-duration-200 desktop-hover:hover:tw-bg-black/80 ${ showDownloadButton ? "tw-pointer-events-auto tw-opacity-100" : "tw-pointer-events-none tw-opacity-0" }`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/media/MediaDisplayVideo.tsx` around lines 163 - 179, The download button remains in the accessibility tree when visually hidden; update the button in MediaDisplayVideo so it is removed from keyboard and AT flow when showDownloadButton is false by adding accessibility attributes: set tabIndex={showDownloadButton ? 0 : -1} and aria-hidden={String(!showDownloadButton)} (or aria-hidden={!showDownloadButton}) and/or hidden={!showDownloadButton} so it cannot be tabbed to or announced; ensure when showDownloadButton is true you restore tabIndex and aria-hidden to make the control reachable again and leave existing onClick/onFocus behavior intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/drops/view/item/content/media/DropListItemContentMediaImage.tsx`:
- Around line 147-157: The div in DropListItemContentMediaImage currently has
role="button", tabIndex={0}, aria-label and key handlers but no action—remove
the button semantics or restore an actionable handler: either delete
role="button", tabIndex and aria-label (and the onKeyDown/onClick
stopPropagation) so it is not focusable, or wire it back to a real handler
(e.g., call the existing view-details function or a provided onOpenFullSize
handler) and ensure onClick and onKeyDown invoke that action (Enter/Space)
instead of only stopPropagation; update the element accordingly in
DropListItemContentMediaImage to keep keyboard/AT behavior correct.
---
Duplicate comments:
In `@components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx`:
- Around line 148-162: The download button is only visually hidden with
tw-opacity-0 and tw-pointer-events-none but remains in the accessibility tree;
update the button rendering in DropListItemContentMediaVideo so when
showDownloadButton is false it is removed from keyboard and assistive technology
access (e.g., add aria-hidden="true" and tabIndex={-1} or render null) and when
true ensure aria-hidden is false and tabIndex is not -1 so handleDownload,
onFocus/showDownloadButtonTemporarily and the ArrowDownTrayIcon are only
reachable when visible; also prevent onFocus logic from running when hidden.
In `@components/drops/view/item/content/media/MediaDisplayVideo.tsx`:
- Around line 163-179: The download button remains in the accessibility tree
when visually hidden; update the button in MediaDisplayVideo so it is removed
from keyboard and AT flow when showDownloadButton is false by adding
accessibility attributes: set tabIndex={showDownloadButton ? 0 : -1} and
aria-hidden={String(!showDownloadButton)} (or aria-hidden={!showDownloadButton})
and/or hidden={!showDownloadButton} so it cannot be tabbed to or announced;
ensure when showDownloadButton is true you restore tabIndex and aria-hidden to
make the control reachable again and leave existing onClick/onFocus behavior
intact.
In `@helpers/media-download.helpers.ts`:
- Around line 8-10: The catch block in helpers/media-download.helpers.ts returns
percent-encoded filenames (variable name) and fallback as-is; change it to
decode the filename before returning by running decodeURIComponent on the
extracted name and on fallback (safely: wrap decodeURIComponent in a try/catch
to fall back to the original value if decoding throws), and return the decoded
name || decoded fallback instead of the raw values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: bfb7f95d-7e41-46dd-bfd2-ebb5531b03b0
📒 Files selected for processing (9)
__tests__/components/drops/view/item/content/media/DropListItemContentMediaVideo.test.tsx__tests__/components/drops/view/item/content/media/MediaDisplayVideo.test.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaImage.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaVideo.tsxcomponents/drops/view/item/content/media/DropMediaActionRow.tsxcomponents/drops/view/item/content/media/MediaDisplayVideo.tsxcomponents/waves/drops/WaveDropPartContentFullWidthImage.tsxcomponents/waves/drops/WaveDropPartContentMedias.tsxhelpers/media-download.helpers.ts
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/utils/select/dropdown/CommonDropdownItemsDefaultWrapper.tsx`:
- Line 147: The dropdown wrapper CommonDropdownItemsDefaultWrapper currently
sets className="tw-absolute tw-z-[1100]" which places all dropdowns above core
modals (z-1000); change the z-index to tw-z-[999] in that wrapper (or
alternatively raise all 1000-level modals to 1101+) so dropdowns do not overlay
modal components like
QuorumProposalDropModal/MemesArtSubmissionModal/WaveLeaderboardCurationDropModal/DropForgeCraftClaimPageClient/NewVersionToast/AllowlistToolCommonModalWrapper.
In `@components/waves/drops/WaveDropPartContentMedias.tsx`:
- Around line 80-82: The computed flag showMediaActionRow currently only checks
media.mime_type and should also respect the disableMediaInteraction prop; update
the expression(s) that compute showMediaActionRow (and the second identical
computation further down) to include a guard like && !disableMediaInteraction so
DropMediaActionRow (and any interactive buttons) are not rendered when
disableMediaInteraction is true; locate the computations in
WaveDropPartContentMedias.tsx (the showMediaActionRow variable(s)) and the
conditional that renders DropMediaActionRow and add the disableMediaInteraction
check there.
In `@helpers/media-download.helpers.ts`:
- Around line 57-60: The downloadMediaUrl helper currently calls fetch(url) with
no timeout; update downloadMediaUrl to use an AbortController and the
ATTACHMENT_DOWNLOAD_FETCH_TIMEOUT_MS timeout pattern used in
DropAttachmentDisplay.tsx: create an AbortController, start a timer that calls
controller.abort() after ATTACHMENT_DOWNLOAD_FETCH_TIMEOUT_MS, pass
controller.signal into fetch(url, { signal }), clear the timer on success, and
throw or rethrow a sensible error when the fetch is aborted or fails so callers
can handle timeouts consistently (retain existing error path for non-timeout
failures).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 7bc14e74-ac1e-42f8-9874-cace5c1092fc
📒 Files selected for processing (11)
__tests__/components/DropListItemContentMediaImage.test.tsx__tests__/components/waves/drops/WaveDropPartContentFullWidthImage.test.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaImage.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaVideo.tsxcomponents/drops/view/item/content/media/DropMediaActionRow.tsxcomponents/drops/view/item/content/media/MediaDisplay.tsxcomponents/drops/view/item/content/media/MediaDisplayVideo.tsxcomponents/utils/select/dropdown/CommonDropdownItemsDefaultWrapper.tsxcomponents/waves/drops/WaveDropPartContentMedias.tsxhelpers/capacitorBlobDownload.helpers.tshelpers/media-download.helpers.ts
💤 Files with no reviewable changes (1)
- components/drops/view/item/content/media/MediaDisplay.tsx
✅ Files skipped from review due to trivial changes (2)
- components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx
- tests/components/DropListItemContentMediaImage.test.tsx
🚧 Files skipped from review as they are similar to previous changes (2)
- components/drops/view/item/content/media/DropListItemContentMediaImage.tsx
- components/drops/view/item/content/media/DropMediaActionRow.tsx
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/waves/drops/WaveDropPartContentFullWidthImage.tsx (1)
131-195:⚠️ Potential issue | 🟡 Minor | ⚡ Quick winReset
isZoomedwhen closing the modal.Now that the reset control is gone, closing the modal while zoomed leaves
isZoomedstucktrue, so the next open starts with stalelimitToBoundsstate.Suggested fix
const handleCloseModal = useCallback(() => { setIsModalOpen(false); + setIsZoomed(false); }, []);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/waves/drops/WaveDropPartContentFullWidthImage.tsx` around lines 131 - 195, The modal's isZoomed state can remain true after closing, causing stale limitToBounds on next open; update the close logic by resetting isZoomed to false when the modal is closed — add a call to setIsZoomed(false) inside the existing handleCloseModal (and any other code paths that close the modal, e.g., backdrop click or Escape handler) so TransformWrapper's limitToBounds reflects the default state on reopen.
♻️ Duplicate comments (1)
components/drops/view/item/content/attachments/DropAttachmentDisplay.tsx (1)
711-720:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDon't override the dropdown portal to
tw-z-[1100]here.This reintroduces the stacking regression the wrapper change was meant to avoid: the attachment menu can render above 1000-level modals/overlays and intercept interaction. Remove the override unless this menu must intentionally escape those layers.
Suggested fix
<CommonDropdownItemsDefaultWrapper isOpen={isOpen} setOpen={(open) => { if (!open && isOpen) { onToggle(); } }} buttonRef={buttonRef} - portalClassName="tw-z-[1100]" >🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@components/drops/view/item/content/attachments/DropAttachmentDisplay.tsx` around lines 711 - 720, The dropdown wrapper CommonDropdownItemsDefaultWrapper is explicitly overriding the portal z-index via portalClassName="tw-z-[1100]", which reintroduces stacking issues by letting the attachment menu escape above 1000-level modals; remove the portalClassName prop (or set it to undefined/omit it) from the CommonDropdownItemsDefaultWrapper invocation so it no longer forces tw-z-[1100] and allows the wrapper's default layering behavior to prevent the menu from intercepting interactions with higher-level overlays.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@components/waves/drops/WaveDropPartContentFullWidthImage.tsx`:
- Around line 131-195: The modal's isZoomed state can remain true after closing,
causing stale limitToBounds on next open; update the close logic by resetting
isZoomed to false when the modal is closed — add a call to setIsZoomed(false)
inside the existing handleCloseModal (and any other code paths that close the
modal, e.g., backdrop click or Escape handler) so TransformWrapper's
limitToBounds reflects the default state on reopen.
---
Duplicate comments:
In `@components/drops/view/item/content/attachments/DropAttachmentDisplay.tsx`:
- Around line 711-720: The dropdown wrapper CommonDropdownItemsDefaultWrapper is
explicitly overriding the portal z-index via portalClassName="tw-z-[1100]",
which reintroduces stacking issues by letting the attachment menu escape above
1000-level modals; remove the portalClassName prop (or set it to undefined/omit
it) from the CommonDropdownItemsDefaultWrapper invocation so it no longer forces
tw-z-[1100] and allows the wrapper's default layering behavior to prevent the
menu from intercepting interactions with higher-level overlays.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 56dbccfe-8569-4efd-8dd2-e900e7797538
📒 Files selected for processing (8)
components/drops/view/item/content/attachments/DropAttachmentDisplay.tsxcomponents/drops/view/item/content/media/DropListItemContentMediaVideo.tsxcomponents/drops/view/item/content/media/DropMediaActionRow.tsxcomponents/drops/view/item/content/media/MediaDisplayVideo.tsxcomponents/utils/select/dropdown/CommonDropdownItemsDefaultWrapper.tsxcomponents/waves/drops/WaveDropPartContentFullWidthImage.tsxcomponents/waves/drops/WaveDropPartContentMedias.tsxhelpers/media-download.helpers.ts
✅ Files skipped from review due to trivial changes (2)
- components/drops/view/item/content/media/DropListItemContentMediaVideo.tsx
- components/drops/view/item/content/media/DropMediaActionRow.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- helpers/media-download.helpers.ts
- components/drops/view/item/content/media/MediaDisplayVideo.tsx
- components/waves/drops/WaveDropPartContentMedias.tsx
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@components/drops/view/item/content/attachments/DropAttachmentDisplay.tsx`:
- Around line 873-879: The catch block in DropAttachmentDisplay.tsx currently
returns early when isCapacitor is true, silently swallowing failures from
fetch/blob()/shareFetchedBlobInNativeApp(); change it so only abort-related
conditions (controller.signal.aborted or downloadAbortRef.current !==
controller) cause an early return, and for other errors (including native share
failures) propagate a visible failure: log the error, clear downloadAbortRef,
stop the spinner state, and surface a user-facing fallback or error (e.g., call
shareFetchedBlobInNativeApp() fallback to a web download function or invoke the
component's error handler/toast via the existing
onDownloadError/setDownloadError or similar). Ensure you reference the same
symbols (controller.signal.aborted, downloadAbortRef.current, isCapacitor,
fetch/blob(), shareFetchedBlobInNativeApp()) when updating the logic so native
failures no longer silently disappear.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: a083eef7-fe32-4861-a3e2-a33082d2da74
📒 Files selected for processing (2)
components/drops/view/item/content/attachments/DropAttachmentDisplay.tsxcomponents/waves/drops/WaveDropPartContentFullWidthImage.tsx
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary by CodeRabbit
New Features
Changes
Documentation
Tests