feat: Long-press artwork context menu for Android #11359

olerichter00 · 2025-01-09T09:12:07Z

This PR resolves ONYX-1474

Description

This implements the Artwork card long-press context menu for Android using a bottom sheet.

Feature Flag: AREnableArtworkCardContextMenuAndroid

Screenshots

Android

iOS

PR Checklist

I have tested my changes on iOS and Android.
I hid my changes behind a feature flag, or they don't need one.
I have included screenshots or videos, or I have not changed the UI.
I have added tests, or my changes don't require any.
I added an app state migration, or my changes do not require one.
I have documented any follow-up work that this PR will require, or it does not require any.
I have added a changelog entry below, or my changes do not require one.

To the reviewers 👀

I would like at least one of the reviewers to run this PR on the simulator or device.

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Artwork card long-press context menu for Android - ole

Dev changes

Need help with something? Have a look at our docs, or get in touch with us.

olerichter00 · 2025-01-24T17:53:26Z

src/app/Components/ArtworkRail/ArtworkRailCard.tsx

+        <Box pr={2}>
+          <RouterLink


Moving ContextMenuArtwork inside RouterLink so RouterLink's onPress still works.

kind of surprised it interferes since I thought it only listened for long press, but if it is necessary it is necessary

I've tried a few combinations, and having a TouchableHighlight for the long press within the Touchable component from palette-mobile for the normal press was the only combination that worked. 🤷

thanks for explaining

ArtsyOpenSource · 2025-01-24T17:56:31Z

This PR contains the following changes:

Android user-facing changes (Artwork card long-press context menu for Android - ole - olerichter00)

Generated by 🚫 dangerJS against f3ce05f

olerichter00 · 2025-01-24T17:56:40Z

src/app/Components/ContextMenu/__tests__/ContextMenuArtwork.tests.tsx

+    `,
+  })
+
+  describe("on Android", () => {


I was not able to test the context menu on iOS with react-native-context-menu-view.

You can try to mock the module with returning a View as a ContextMenu

I gave it a try, but it's not so easy to mock the ContextMenu component because it's not only rendering children but also taking an array of actions to render and managing the long press behavior. So simply mocking it with View, unfortunately, doesn't give us anything to test :/

brainbicycle

looks good to me!

MounirDhahri

small comment but nothing blocking 👏

MounirDhahri · 2025-01-27T16:59:40Z

src/app/Components/ArtworkRail/ArtworkRailCard.tsx

+        <Box pr={2}>
+          <RouterLink


thanks for explaining

MounirDhahri · 2025-01-27T17:02:08Z

src/app/Components/ContextMenu/ContextMenuArtwork.tsx

 }
+
+const artworkFragment = graphql`


nitpick: can we make this call when the context menu or the bottom sheet shows up to avoid calling this data when it's not required

That could be something to try out! I will merge the PR as it is right now and give it a try to lazy load the data for the context menu on long press.

It might work but it could lead to issues with how the opening animation works for the context menu 🤔

MounirDhahri · 2025-01-27T17:03:02Z

src/app/Components/ContextMenu/__tests__/ContextMenuArtwork.tests.tsx

+    `,
+  })
+
+  describe("on Android", () => {


You can try to mock the module with returning a View as a ContextMenu

artsy-peril bot added the Jira Synced label Jan 9, 2025

olerichter00 changed the title ~~feat: Long-press artwork context menu for Android~~ feat(WIP): Long-press artwork context menu for Android Jan 9, 2025

olerichter00 marked this pull request as draft January 9, 2025 09:12

olerichter00 self-assigned this Jan 9, 2025

olerichter00 force-pushed the olerichter00/ONYX-1474/long-press-artwork-context-menu-for-android branch from 87895ca to c44814b Compare January 24, 2025 17:49

olerichter00 mentioned this pull request Jan 24, 2025

chore: Add AREnableArtworkCardContextMenuAndroid artsy/echo#761

Closed

olerichter00 commented Jan 24, 2025

View reviewed changes

olerichter00 changed the title ~~feat(WIP): Long-press artwork context menu for Android~~ feat: Long-press artwork context menu for Android Jan 24, 2025

olerichter00 marked this pull request as ready for review January 24, 2025 17:53

olerichter00 commented Jan 24, 2025

View reviewed changes

olerichter00 force-pushed the olerichter00/ONYX-1474/long-press-artwork-context-menu-for-android branch from ccd90a0 to 9aea512 Compare January 27, 2025 10:30

olerichter00 requested review from MounirDhahri and brainbicycle January 27, 2025 10:43

brainbicycle previously approved these changes Jan 27, 2025

View reviewed changes

MounirDhahri previously approved these changes Jan 27, 2025

View reviewed changes

olerichter00 added 5 commits January 28, 2025 09:45

feat: Long-press artwork context menu for Android

f5a62a8

ff, tests and touchable fix

8fe3aca

ff

c28a62a

fix the logic

b4e25b0

safe area

f3ce05f

olerichter00 dismissed stale reviews from MounirDhahri and brainbicycle via f3ce05f January 28, 2025 08:55

olerichter00 force-pushed the olerichter00/ONYX-1474/long-press-artwork-context-menu-for-android branch from 4e0d984 to f3ce05f Compare January 28, 2025 08:55

olerichter00 added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label Jan 28, 2025

artsy-peril bot merged commit 72dbe24 into main Jan 28, 2025
7 checks passed

artsy-peril bot deleted the olerichter00/ONYX-1474/long-press-artwork-context-menu-for-android branch January 28, 2025 09:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Long-press artwork context menu for Android #11359

feat: Long-press artwork context menu for Android #11359

olerichter00 commented Jan 9, 2025 •

edited

Loading

olerichter00 Jan 24, 2025

brainbicycle Jan 27, 2025

olerichter00 Jan 27, 2025

MounirDhahri Jan 27, 2025

ArtsyOpenSource commented Jan 24, 2025 •

edited

Loading

olerichter00 Jan 24, 2025

MounirDhahri Jan 27, 2025

olerichter00 Jan 28, 2025

brainbicycle left a comment

MounirDhahri left a comment

MounirDhahri Jan 27, 2025

MounirDhahri Jan 27, 2025

olerichter00 Jan 28, 2025

MounirDhahri Jan 27, 2025

feat: Long-press artwork context menu for Android #11359

feat: Long-press artwork context menu for Android #11359

Conversation

olerichter00 commented Jan 9, 2025 • edited Loading

Description

Screenshots

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ArtsyOpenSource commented Jan 24, 2025 • edited Loading

This PR contains the following changes:

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

brainbicycle left a comment

Choose a reason for hiding this comment

MounirDhahri left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

olerichter00 commented Jan 9, 2025 •

edited

Loading

ArtsyOpenSource commented Jan 24, 2025 •

edited

Loading