Gene page refactor! #413

alloy · 2017-12-12T00:14:43Z

This separates the artists and artworks parts from the gene page.

Signed-off-by: Eloy Durán <[email protected]>

See https://stackoverflow.com/questions/42090191/picks-k-type-with-dynamic-computed-keys/42131589#42131589

alloy · 2017-12-12T01:02:04Z

Btw, there’s probably a few more places in Force where Reaction components are used that require the context setup.

mzikherman · 2017-12-12T01:28:17Z

src/Components/Gene/NewContents.tsx

@@ -36,7 +36,7 @@ class GeneNewContents extends React.Component<Props, State> {

  onDropdownSelect(slice: string, value: string) {
    this.setState({
-      [slice.toLowerCase()]: value,
+      [slice.toLowerCase() as any]: value,


Yep 😝

In short, Pick currently only works when literals are used as keys, or of course ‘anything’. See the linked SO Q/A and the TS issue the answer links to https://stackoverflow.com/questions/42090191/picks-k-type-with-dynamic-computed-keys/42131589#42131589

Wow, super informative, thanks!

mzikherman · 2017-12-12T03:05:06Z

As a note (to myself) on what is still left to do in this PR:

'artworks' mode:
- ~~Wire up being able to click on the filter dropdowns~~
- ~~Wire up headline display~~
- ~~Wire up being able to change the sort~~
- ~~Wire up for sale toggle~~
- ~~Wire up switching to 'artists' mode~~
- Inf scroll/load more

Then on Force, the code which initializes the Reaction component should update to support the new style: rendering this component and passing in the gene props from the bootstrapped sharify data.

alloy · 2017-12-12T03:11:23Z

@mzikherman Here’s an example I could easily find that also needs updating to use a QueryRenderer https://github.com/artsy/force/blob/7f8984a814e2d9a8895330826f0f2802c69f0ba7/desktop/apps/tag/client.js. That might actually be the only other place.

mzikherman · 2017-12-12T03:16:31Z

Yup, I totally forgot about the tag page but I now recall those two (gene + tag) being the places where the new filter + bricks were being used. I can doublecheck but I yea, I think those are the only two.

joeyAghion · 2017-12-13T15:09:16Z

Thanks @mzikherman for linking me to this. I didn't realize this was happening with the Relay upgrade.

Since there were several of issues with the current gene page implementation (artsy/force#1823, artsy/force#1822, artsy/force#1821), this is a great and rare opportunity to carefully review the actual requests that cascade from the page (including requests from Force's client-side, from its server-side, from MP, and so on). We have new MP response properties, Datadog traces, and staging logs to help with this.

alloy

I guess the attempt at using an interface for the artwork edge only still compiled because the interface is still present in your local MP schema?

alloy · 2017-12-14T02:10:52Z

src/Components/ArtworkFilter/index.tsx

            pageInfo {
              hasNextPage
              endCursor
            }
            edges {
+            ...ArtworkGrid_artworks


Oh, these attempts at using an interface will have to be reverted!

alloy · 2017-12-14T02:11:28Z

package.json

@@ -22,7 +22,7 @@
    "test:watch": "jest --watch --runInBand",
    "storybook": "node verify-node-version.js && concurrently --kill-others 'yarn relay --watch' 'env GRAPHQL_NO_NAME_WARNING=true start-storybook -p 9001'",
    "deploy-storybook": "yarn relay && NODE_ENV=production storybook-to-ghpages",
-    "sync-schema": "cd externals/metaphysics && git fetch && git checkout origin/master && yarn install && npm run dump-schema -- ../../data",
+    "sync-schema": "cd externals/metaphysics && git fetch && git checkout origin/pagination_fuckery && yarn install && npm run dump-schema -- ../../data",


:{{ will undo

alloy · 2017-12-14T02:11:55Z

src/Components/ArtworkGrid.tsx

@@ -51,7 +51,7 @@ export class ArtworkGrid extends React.Component<Props, State> {
  sectionedArtworks() {
    const sectionedArtworks: ArtworkRelayProps[][] = []
    const sectionRatioSums = []
-    const artworks = this.props.artworks ? this.props.artworks.edges : []
+    const artworks = this.props.artworks ? this.props.artworks : []


Needs to be reverted.

alloy · 2017-12-14T02:12:03Z

src/Components/ArtworkGrid.tsx

}
+

Needs to be reverted.

alloy · 2017-12-14T02:12:38Z

src/Components/Gene/Contents.tsx

              node {
                __id
              }
            }
-            ...ArtworkGrid_artworks
+


Needs to be reverted.

alloy · 2017-12-14T02:12:47Z

src/Components/__stories__/ArtworkGrid.story.tsx

-              ...ArtworkGrid_artworks
+              edges {
+                ...ArtworkGrid_artworks
+              }


Needs to be reverted.

alloy · 2017-12-14T02:13:21Z

src/Components/Onboarding/Steps/Genes/SuggestedGenes.tsx

-      image_url
+      image {
+        url
+      }


This will probably lead to a merge error with work that @xtina-starr and @yuki24 have been doing.

alloy · 2017-12-14T21:08:44Z

src/Components/__stories__/Gene.story.tsx

-    return (
-      <div>
-        <GeneExample geneID="old-master-influenced-fantasy" />
+        < ContextProvider currentUser={{id: "58ed621f9c18db55bc6b2dde", name: "Matt", accessToken: "eyJ0eXAiOiJKV1QiLCJhbGciOiJIUzI1NiJ9.eyJzdWIiOiI1OGVkNjIxZjljMThkYjU1YmM2YjJkZGUiLCJzYWx0X2hhc2giOiI2ZTVkMWVhYzJmMWQzNDhmZmZmOGJmMDAyOGQ1OTViOCIsInJvbGVzIjoidXNlciIsInBhcnRuZXJfaWRzIjpbXSwiZXhwIjoxNTE3NzYwMjA0LCJpYXQiOjE1MTI1NzYyMDQsImF1ZCI6IjRlMzZlZmE0ZGI0ZTMyMDAwMTAwMDM1OSIsImlzcyI6IkdyYXZpdHkiLCJqdGkiOiI1YTI4MTRjYzhiM2I4MTQ1ZjI2YTliNzUifQ.ksCRUaQRvGNRW2VnOSAAT32r9PM9s04axn7SqI-_Kno"}}>


Still got a token in there :)

mzikherman · 2017-12-14T22:37:41Z

src/__stories__/config.js

-  sortStoriesByKind: true,
-})
+
+// TODO: Fix the below


@orta this seems to be broken with the latest version of storybook- would you be able to look into it?

alloy assigned mzikherman Dec 12, 2017

[Gene] Seperate artists and artworks.

d1d2aa5

Signed-off-by: Eloy Durán <[email protected]>

alloy force-pushed the gene-page-refactor branch from 2a7221c to d1d2aa5 Compare December 12, 2017 00:16

[Gene] Fix type issue with dynamic object key typing.

77b544d

See https://stackoverflow.com/questions/42090191/picks-k-type-with-dynamic-computed-keys/42131589#42131589

mzikherman reviewed Dec 12, 2017

View reviewed changes

mzikherman added 4 commits December 12, 2017 13:04

wip- artworks filtering and headline

e28ad16

wip- artwork sorting working

60a1810

wip- artwork for sale toggle working

ba26a47

wip- artist mode from artworks

3cdac43

wip- artwork load more works!

8c91417

alloy commented Dec 14, 2017

View reviewed changes

mzikherman added 2 commits December 14, 2017 14:08

cleanup of gene

e46b6fd

Remove root level filter component

dabd6ea

alloy commented Dec 14, 2017

View reviewed changes

mzikherman added 4 commits December 14, 2017 16:29

Add tag page

300bf1c

merge

16fe9ff

Callback for state change on gene and tag filter

17930d3

Remove staging credentials

a89ff30

mzikherman changed the title ~~[WIP] Gene page refactor~~ Gene page refactor! Dec 14, 2017

mzikherman reviewed Dec 14, 2017

View reviewed changes

mzikherman merged commit 9a53fe2 into relay-modern Dec 14, 2017

mzikherman deleted the gene-page-refactor branch December 14, 2017 22:41

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Gene page refactor! #413

Gene page refactor! #413

alloy commented Dec 12, 2017

alloy commented Dec 12, 2017

mzikherman Dec 12, 2017

alloy Dec 12, 2017

mzikherman Dec 12, 2017

mzikherman commented Dec 12, 2017 •

edited

Loading

alloy commented Dec 12, 2017

mzikherman commented Dec 12, 2017

joeyAghion commented Dec 13, 2017

alloy left a comment

alloy Dec 14, 2017

alloy Dec 14, 2017

mzikherman Dec 14, 2017

alloy Dec 14, 2017

alloy Dec 14, 2017

alloy Dec 14, 2017

alloy Dec 14, 2017

alloy Dec 14, 2017

alloy Dec 14, 2017

mzikherman Dec 14, 2017

Gene page refactor! #413

Gene page refactor! #413

Conversation

alloy commented Dec 12, 2017

alloy commented Dec 12, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mzikherman commented Dec 12, 2017 • edited Loading

alloy commented Dec 12, 2017

mzikherman commented Dec 12, 2017

joeyAghion commented Dec 13, 2017

alloy 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

Choose a reason for hiding this comment

mzikherman commented Dec 12, 2017 •

edited

Loading