feat(EMI-2164): Artsy shipping estimate widget - initial implementation #15126

erikdstock · 2025-01-28T20:46:36Z

The type of this PR is: FEAT
This PR solves EMI-2164

Description

This PR is the initial implementation of the shipping estimate widget to be shown in the shipping & taxes section of an artwork page. It does not contain widget styling and will need some more changes to work fully.

Note on context provider
Due to specifics of how we load the browser-only shipping estimate module, it is not available at the moment the page renders. This makes the loading async, a problem that snowballs because the widget is hidden in a Collapsing element. My initial solution was to create a whole context to load the module outside of the Collapse, but there is still an async-ness to loading the widget link itself, so it might be worth removing this extra complexity for the two separate layers of loading.

A/C

Attempt to load the widget on artworks with global (domestic/international) artsy shipping enabled
Support artworks with no edition sets
There are still console.log()s in the code - will remove as I finish some of the investigations below
If required artwork metadata is missing, render a normal fallback shipping availability message (partially complete, want a little more of a guard)
Tests (initial tests are done)

Unknowns & followup work

~~Depends on chore: consistent artwork money fields resolver metaphysics#6367~~
artwork weight may be required
artwork location extraction is hacky
[Ticketed] Support artworks with a single edition set, but not multiple (may be added to a future ticket)

- Load widget with skeleton and fallback text - Still lacking some MP data

relativeci · 2025-01-28T20:48:10Z

#1640 Bundle Size — 9.01MiB (+0.74%).

4374b12(current) vs 56d5d43 main#1636(baseline)

Warning

Bundle contains 14 duplicate packages – View duplicate packages

Bundle metrics

4 changes

1 regression

	Current #1640	Baseline #1636
Initial JS	`3.65MiB`(`+0.78%`)	`3.62MiB`
Initial CSS	`0B`	`0B`
Cache Invalidation	`97.74%`	`50.38%`
Chunks	`105`	`105`
Assets	`108`	`108`
Modules	`5851`(`+0.02%`)	`5850`
Duplicate Modules	`523`	`523`
Duplicate Code	`4%`(`+0.25%`)	`3.99%`
Packages	`266`	`266`
Duplicate Packages	`13`	`13`

Bundle size by type

2 changes

2 regressions

	Current #1640	Baseline #1636
JS	`8.87MiB` (`+100%`)	`undefined`
Other	`147.32KiB` (`+100%`)	`undefined`

Bundle analysis report Branch erik.artsy-shipping-widget Project dashboard

^{Generated by RelativeCI Documentation Report issue}

erikdstock · 2025-01-28T20:49:12Z

src/Components/ArtsyShippingEstimate.tsx

+const ARTWORK_FRAGMENT = graphql`
+  fragment ArtsyShippingEstimate_artwork on Artwork {
+    isFramed
+    category # Raw category required for mapping to Arta subtype
+    shippingOrigin
+    shippingCountry
+    location {
+      country
+      postalCode
+      city
+    }
+    priceCurrency
+    priceListed {
+      major
+    }
+    heightCm
+    widthCm
+    # shippingWeight # may need to be added?
+  }
+`


Problems with our schema:

category is deprecated but used to map to values required for estimate

weight not accessible

location not accessible, so using shippingOrigin

priceListed does not include major (see chore: consistent artwork money fields resolver metaphysics#6367)

oxaudo

Those are immediate comments but maybe worth pairing on it tomorrow AM.

oxaudo · 2025-01-28T20:59:39Z

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebar.tsx

-        </Text>
-      )}
-      <ArtworkSidebarArtistsFragmentContainer artwork={artwork} />
+    <ArtsyShippingEstimateProvider>


Oh... This seems off to me that the whole sidebar is wrapped into this provider. Wonder if we can make it more local and only wrap the relevant section?

I'll remove the provider, agree it's not needed here

oxaudo · 2025-01-28T21:07:00Z

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarShippingInformation.tsx

  const { trackEvent } = useTracking()
+  const globalArtsyShipping =
+    !!artwork.artsyShippingDomestic || !!artwork.artsyShippingInternational


Don't think this is an accurate condition that we want here. Believe it has to be !!artwork.artsyShippingDomestic && (!!artwork.artsyShippingInternational || (!artwork.artsyShippingInternationa && !artwork.international_shipping_fee)). So either arta shipping both domestic and international or arta domestic and international not configured.

Let's follow up on this in person, i believe you have the expertise and it's a small change if so

oxaudo · 2025-01-28T21:08:01Z

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarShippingInformation.tsx

 import { useTracking } from "react-tracking"

 export interface ShippingInformationProps {
  artwork: ArtworkSidebarShippingInformation_artwork$data
 }
+const CALCULATED_IN_CHECKOUT_MESSAGE = "Shipping: Calculated in checkout"


This is not the case any more after this pr: artsy/metaphysics#6383

Removing for now bc designs don't need it

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarShippingInformation.tsx

starsirius · 2025-01-30T03:44:50Z

src/Components/ArtsyShippingEstimate.tsx

+const artworkValue = (artwork: ShippableArtwork): ArtworkValue | null => {
+  if (!!artwork.heightCm && !!artwork.widthCm) {
+    return {
+      value: artwork.priceListed?.major || 500, // TODO: get a real value


I think price_listed is the backend price that is not exposed publicly so I guess users will not be able to see this. Also, do we aim to support artworks displayed as price range, etc? (sorry if I missed the product requirements)

Good point @starsirius! I believe there was not a discussion about excluding range works. And I forgot about this whole 'price being private' for MO sometimes. Think taking midpoint makes sense here but def something to discuss.

Perhaps this is the right field for public facing pricing. Confusing indeed!

- dont render if estimate creation fails - earlier check for feature flag - close widget when navigating away - initial widget link styling

oxaudo

I feel good now if we merge this and continue iterating. Comments are mostly to remember where some clean up needed.

oxaudo · 2025-01-30T19:08:37Z

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebar.tsx

+            label={
+              <Flex flexDirection="column" justifyContent="flex-start">
+                <Text>Shipping and taxes</Text>
+                {globalArtsyShipping && artsyShippingEstimateEnabled && (


Like that all the guarding is scoped to this one place! Thanks for the update.

oxaudo · 2025-01-30T19:11:49Z

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebar.tsx

+                )}
+              </Flex>
+            }
+          >
            <ArtworkSidebarShippingInformationFragmentContainer


As discussed in person - we are not hiding the actual shippingInfo line in expanded section of this view. We can definitely do it - just don't think it's adding anything to the test and believe keeping code less intermingled with the estimate test here is a good goal. Might still be worth confirming out assumption with Cami and Vivi.

src/Apps/Artwork/Components/ArtworkSidebar/ArtworkSidebarShippingInformation.tsx

oxaudo · 2025-01-30T19:18:14Z

src/Components/ArtsyShippingEstimate.tsx

+    }
+
+    const loadEstimate = async () => {
+      console.log("***", "oading estimate", estimateInput)


oxaudo · 2025-01-30T19:22:08Z

Spec fails are legit though and should be fixed before merging.

src/Components/ArtsyShippingEstimate.tsx

damassi

The current refactor / simplification is much improved 👍

erikdstock · 2025-01-30T20:19:56Z

package.json

@@ -189,6 +189,7 @@
    "yup": "0.32.6"
  },
  "devDependencies": {
+    "@artaio/arta-browser": "^2.16.1",


Added here for the types - the lib is downloaded by script.

erikdstock added 2 commits January 28, 2025 14:29

initial widget setup

5ceec81

marshall full arta types

858d98b

- Load widget with skeleton and fallback text - Still lacking some MP data

erikdstock added the Do not merge label Jan 28, 2025

erikdstock requested a review from oxaudo January 28, 2025 20:46

erikdstock self-assigned this Jan 28, 2025

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

erikdstock commented Jan 28, 2025

View reviewed changes

oxaudo reviewed Jan 28, 2025

View reviewed changes

erikdstock added 3 commits January 28, 2025 17:44

refactor: no context provider

43d5da0

refactor: useLoadScript to get arta widget lib

7febfa4

chore: render widget link in heading of shipping + tax section

f6d63fd

starsirius reviewed Jan 30, 2025

View reviewed changes

erikdstock added 2 commits January 30, 2025 12:23

final tweaks:

95ec5a9

- dont render if estimate creation fails - earlier check for feature flag - close widget when navigating away - initial widget link styling

style tweaks

6b5d6cf

oxaudo previously approved these changes Jan 30, 2025

View reviewed changes

damassi reviewed Jan 30, 2025

View reviewed changes

src/Components/ArtsyShippingEstimate.tsx Outdated Show resolved Hide resolved

damassi reviewed Jan 30, 2025

View reviewed changes

src/Components/ArtsyShippingEstimate.tsx Outdated Show resolved Hide resolved

damassi reviewed Jan 30, 2025

View reviewed changes

src/Components/ArtsyShippingEstimate.tsx Outdated Show resolved Hide resolved

damassi reviewed Jan 30, 2025

View reviewed changes

src/Components/ArtsyShippingEstimate.tsx Outdated Show resolved Hide resolved

damassi previously approved these changes Jan 30, 2025

View reviewed changes

fix tests

50838b6

erikdstock dismissed stale reviews from damassi and oxaudo via 50838b6 January 30, 2025 19:35

erikdstock added 3 commits January 30, 2025 13:36

remove extra relay props

0ea6001

add test for negative case

73e297c

respond to feedback

4374b12

erikdstock commented Jan 30, 2025

View reviewed changes

erikdstock merged commit d53b113 into main Jan 30, 2025
11 checks passed

erikdstock deleted the erik.artsy-shipping-widget branch January 30, 2025 20:22

artsy-peril bot mentioned this pull request Jan 30, 2025

Deploy #15130

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(EMI-2164): Artsy shipping estimate widget - initial implementation #15126

feat(EMI-2164): Artsy shipping estimate widget - initial implementation #15126

erikdstock commented Jan 28, 2025 •

edited

Loading

relativeci bot commented Jan 28, 2025 •

edited

Loading

erikdstock Jan 28, 2025

oxaudo left a comment

oxaudo Jan 28, 2025

erikdstock Jan 28, 2025

oxaudo Jan 28, 2025

erikdstock Jan 29, 2025

oxaudo Jan 28, 2025

erikdstock Jan 29, 2025

starsirius Jan 30, 2025

oxaudo Jan 30, 2025

starsirius Jan 30, 2025

oxaudo left a comment

oxaudo Jan 30, 2025

oxaudo Jan 30, 2025

oxaudo Jan 30, 2025

oxaudo commented Jan 30, 2025

damassi left a comment

erikdstock Jan 30, 2025

feat(EMI-2164): Artsy shipping estimate widget - initial implementation #15126

feat(EMI-2164): Artsy shipping estimate widget - initial implementation #15126

Conversation

erikdstock commented Jan 28, 2025 • edited Loading

Description

relativeci bot commented Jan 28, 2025 • edited Loading

#1640 Bundle Size — 9.01MiB (+0.74%).

Choose a reason for hiding this comment

oxaudo 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

Choose a reason for hiding this comment

oxaudo 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

oxaudo commented Jan 30, 2025

damassi left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

erikdstock commented Jan 28, 2025 •

edited

Loading

relativeci bot commented Jan 28, 2025 •

edited

Loading