feat: add galleries near you rail #10571

MounirDhahri · 2024-08-07T15:58:22Z

This PR resolves ONYX-1244

Description

This PR adds a galleries near you rail to the home view.

Screen.Recording.2024-08-30.at.11.12.11.mov

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

add galleries near you rail - mounir

iOS user-facing changes

Android user-facing changes

Dev changes

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

ArtsyOpenSource · 2024-08-30T10:24:45Z

This PR contains the following changes:

Cross-platform user-facing changes (add galleries near you rail - mounir)

Generated by 🚫 dangerJS against 700aaee

olerichter00

✨ looks great! I would just remove the default view all href because I'd suggest to only define the copy and data in MP and not in the client.

olerichter00 · 2024-08-30T11:32:20Z

src/app/Scenes/HomeView/HomeView.tsx

@@ -117,6 +117,10 @@ const sectionsFragment = graphql`
              internalID
              ...SalesRailHomeViewSection_section
            }
+            ... on GalleriesHomeViewSection {
+              internalID


Fan we move internalID outside the ... on to avoid duplication?

Does the union upper level have an internalID available?

It wasn't possible 😢 I tried that, it's only defined in individual sections

olerichter00 · 2024-08-30T11:35:15Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tsx

+                variant={hasImage ? "outlineLight" : "fillDark"}
+                size="small"
+                onPress={() => {
+                  const href = section.component?.behaviors?.viewAll?.href ?? "/galleries-for-you"


What do you think about removing the default? Like this we would be able to add a section without view all link.

We could move the default to MP to make sure the href is always set if we need to.

yes, that makes sense. I can do that

araujobarret · 2024-08-30T12:32:07Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tests.tsx

+describe("GalleriesHomeViewSection", () => {
+  const { renderWithRelay } = setupTestWrapper<GalleriesHomeViewSectionTestsQuery>({
+    Component: (props) => {
+      if (!props.homeView.section) {


consider adding the annotation to the graphql @required(action: NONE) so you don't need this extra check here 🌮

thanks - I didn't consider that here

MounirDhahri · 2024-08-30T12:54:12Z

@olerichter00 I am not sure what you mean by moving the data to MP - I assumed it's already there now

olerichter00 · 2024-08-30T13:22:44Z

@olerichter00 I am not sure what you mean by moving the data to MP - I assumed it's already there now

Sorry, this is a bit ambiguous. I was referring to this comment:#10571 (comment)

olerichter00 · 2024-08-30T13:23:47Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tsx

+            <Flex mt={0.5} maxWidth={150}>
+              <Button
+                variant={hasImage ? "outlineLight" : "fillDark"}
+                size="small"
+                onPress={() => {
+                  const href = section.component?.behaviors?.viewAll?.href
+                  if (href) {
+                    navigate(href)
+                  }
+                }}
+              >
+                Explore
+              </Button>
+            </Flex>


We should hide the button if we don't have a view all link 👻

Suggested change

<Flex mt={0.5} maxWidth={150}>

<Button

variant={hasImage ? "outlineLight" : "fillDark"}

size="small"

onPress={() => {

const href = section.component?.behaviors?.viewAll?.href

if (href) {

navigate(href)

}

}}

>

Explore

</Button>

</Flex>

{!!viewAllHref && <Flex mt={0.5} maxWidth={150}>

<Button

variant={hasImage ? "outlineLight" : "fillDark"}

size="small"

onPress={() => {

navigate(viewAllHref)

}}

>

Explore

</Button>

</Flex>

aha I got what you mean now

olerichter00 · 2024-08-30T13:26:41Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tsx

+              </Text>
+            </Flex>
+
+            <Flex mt={0.5} maxWidth={150}>


Can we remove the max width? It's mostly problematic when setting absolute values, especially when screen sizes can vary.

Increased font sizes could potentially cause issues.

I am copying the same exact styling from the existing element here without adjusting the styles.

oh actually no- I added that later to make sure the text remains visible

True, it was already there. I'll leave it to you if you want to update it.

150 pixels was enough even in screens with huge fonts and up to 2 words

olerichter00 · 2024-08-30T13:27:44Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tsx

+                  }
+                }}
+              >
+                Explore


What do you think about moving the button copy to Metaphysics as well (e.g. section.component?.behaviors?.viewAll?.text)? Like this, we could modify it easily and even have different copies for different components.

Ah, we already query for it here 👀

Aha I missed this - I already defined it in MP

olerichter00

A few more comments 🐌

nickskalkin

Awesome! Found a typo 👇

nickskalkin · 2024-08-30T15:04:12Z

src/app/Scenes/HomeView/Sections/GalleriesHomeViewSection.tsx

+  const hasImage = !!section.component.backgroundImageURL
+  const textColor = hasImage ? "white100" : "black100"
+
+  const componenntHref = section.component?.href


MounirDhahri added the Art Advisor label Aug 7, 2024

MounirDhahri self-assigned this Aug 7, 2024

MounirDhahri force-pushed the feat/add-galleries-home-view-section branch from 8bed90d to 553da60 Compare August 30, 2024 09:10

artsy-peril bot added the Jira Synced label Aug 30, 2024

MounirDhahri requested review from nickskalkin, olerichter00 and anandaroop August 30, 2024 10:22

MounirDhahri marked this pull request as ready for review August 30, 2024 10:22

MounirDhahri force-pushed the feat/add-galleries-home-view-section branch from fde9355 to 7b0ab52 Compare August 30, 2024 10:32

olerichter00 previously approved these changes Aug 30, 2024

View reviewed changes

araujobarret previously approved these changes Aug 30, 2024

View reviewed changes

MounirDhahri dismissed stale reviews from araujobarret and olerichter00 via e03c92e August 30, 2024 12:46

MounirDhahri force-pushed the feat/add-galleries-home-view-section branch from e03c92e to 53f96be Compare August 30, 2024 12:54

olerichter00 reviewed Aug 30, 2024

View reviewed changes

olerichter00 previously approved these changes Aug 30, 2024

View reviewed changes

MounirDhahri dismissed olerichter00’s stale review via 1a012a8 August 30, 2024 13:28

nickskalkin previously approved these changes Aug 30, 2024

View reviewed changes

MounirDhahri added the Squash On Green A label to indicate that Peril should squash-merge this PR when all statuses are green label Aug 30, 2024

MounirDhahri dismissed nickskalkin’s stale review via 170f000 August 30, 2024 15:37

MounirDhahri added 5 commits September 3, 2024 15:25

feat: add galleries near you rail

d2f5fb4

chore: add tests

cccf17c

fix: address review comments

8440f0d

fix: address review comment

1c43d2e

fix: typo

700aaee

MounirDhahri force-pushed the feat/add-galleries-home-view-section branch from 170f000 to 700aaee Compare September 3, 2024 13:26

artsy-peril bot merged commit 89f39e9 into main Sep 3, 2024
7 checks passed

artsy-peril bot deleted the feat/add-galleries-home-view-section branch September 3, 2024 13:33

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add galleries near you rail #10571

feat: add galleries near you rail #10571

MounirDhahri commented Aug 7, 2024 •

edited by jira bot

Loading

ArtsyOpenSource commented Aug 30, 2024 •

edited

Loading

olerichter00 left a comment

olerichter00 Aug 30, 2024

araujobarret Aug 30, 2024

MounirDhahri Aug 30, 2024

olerichter00 Aug 30, 2024

olerichter00 Aug 30, 2024

MounirDhahri Aug 30, 2024

araujobarret Aug 30, 2024

MounirDhahri Aug 30, 2024

MounirDhahri commented Aug 30, 2024

olerichter00 commented Aug 30, 2024

olerichter00 Aug 30, 2024 •

edited

Loading

MounirDhahri Aug 30, 2024

olerichter00 Aug 30, 2024

MounirDhahri Aug 30, 2024

MounirDhahri Aug 30, 2024

olerichter00 Aug 30, 2024

MounirDhahri Aug 30, 2024

olerichter00 Aug 30, 2024

olerichter00 Aug 30, 2024

MounirDhahri Aug 30, 2024

olerichter00 left a comment

nickskalkin left a comment

nickskalkin Aug 30, 2024

feat: add galleries near you rail #10571

feat: add galleries near you rail #10571

Conversation

MounirDhahri commented Aug 7, 2024 • edited by jira bot Loading

Description

PR Checklist

To the reviewers 👀

Changelog updates

Cross-platform user-facing changes

iOS user-facing changes

Android user-facing changes

Dev changes

ArtsyOpenSource commented Aug 30, 2024 • edited Loading

This PR contains the following changes:

olerichter00 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

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

MounirDhahri commented Aug 30, 2024

olerichter00 commented Aug 30, 2024

olerichter00 Aug 30, 2024 • edited Loading

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

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 left a comment

Choose a reason for hiding this comment

nickskalkin left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

MounirDhahri commented Aug 7, 2024 •

edited by jira bot

Loading

ArtsyOpenSource commented Aug 30, 2024 •

edited

Loading

olerichter00 Aug 30, 2024 •

edited

Loading