Swatch Treatments #1512

dani97 · 2019-08-01T19:27:52Z

Description

Update Swatch Treatments.

Related Issue

Closes #1390 .

Verification Steps

Go to the configurable PDP.
Verify the tool-tips not shown when selecting the options and new treatment comes.
Make sure the same working in cart.

Screenshots / Screen Captures (if appropriate)

Proposed Labels for Change Type/Package

major (e.g x.0.0 - a breaking change)
minor (e.g 0.x.0 - a backwards compatible addition)
patch (e.g 0.0.x - a bug fix)

Checklist:

I have updated the documentation accordingly, if necessary.
I have added tests to cover my changes, if necessary.

PWAStudioBot · 2019-08-01T19:33:12Z

	Messages
📖	Access a deployed version of this PR here. Make sure to wait for the "pwa-pull-request-deploy" job to complete.
📖	DangerCI Failures related to missing labels/description/linked issues/etc will persist until the next push or next nightly build run (assuming they are fixed).

Generated by 🚫 dangerJS against 19c398c

* Converted item component to stateless. * Converted items component to stateless. * Converted list component to stateless. * Removing variable name shortcuts. * useMemo syntax changes. * updateSelection return value change. * Moved functional logic into custom useListState hook. * Syntax change. * Creating handlers only when key changes. * Using const instead of function. * Importing useMemo. * Prettier changes. * Memoizing handleSelectionChange function. * Moving hooks out of loops. * Prettier changes. * Added onSelectionChange to useEffect deps array. * Minor changes. * Implemented onClick and onFocus functions. * Added uniqueID to props to be removed from DOM. * Removing uniqueID from custom props. * Moving useListState custom hook to separate file. * Added jsdocs to useListState custom hook. * Minor change. * Refactored items.spec.js * Refactored list.spec.js * Revert "List component refactor." * fixed eslint issues

sirugh

Leaving you some initial feedback. Please fix the tests as well.

Also I'm out of office next week so someone else on the team will need to pick up the review.

packages/venia-concept/src/components/ProductOptions/tile.js

packages/venia-concept/src/components/ProductOptions/swatch.js

packages/venia-concept/src/components/ProductOptions/option.js

* fixed eslint issues * prettier and tests corrected

options component made as function component and test updated

vercel · 2019-08-07T14:28:28Z

This pull request is automatically deployed with Now.
To access deployments, click Details below or on the icon next to each push.

Latest deployment for this branch: https://pwa-studio-git-fork-dani97-feature-1390-update-swatc-538576.mmansoor.now.sh

…1390-update-swatch-treatments

…ni97/pwa-studio into feature/1390-update-swatch-treatments

dani97 · 2019-08-07T14:46:11Z

@jimbo As per @sirugh comment I refactored options component into function component but I could not fix tests
1 ) does not call onSelectionChange if not provided
2 ) calls onSelectionChange function with optionId and selection
These two tests get options instance and call handleSelectionChange method.
Since I refactored it into function component, I cannot access the instance in test. Please advise on this.

jimbo · 2019-08-07T20:04:54Z

@dani97 Thanks for taking this on. Those tooltips were bothering me for a long time! 😅

I've updated the tests for you; have a look and let me know if you have any questions. Make sure to use createTestInstance from Peregrine, and use the Test Renderer docs to work with the returned instance.

I also made some minor changes to the Option component. In general, it's better to not keep the selection set in multiple components' state, so I swapped selection to be the derived state (the label of the selected option). Should simplify things a bit.

jimbo · 2019-08-07T20:06:12Z

packages/venia-concept/src/components/ProductOptions/option.js

+    const optionType = getOptionType({ attribute_code, values });
+
+    return optionType === 'swatch' ? SwatchList : TileList;
+};


Note that I extracted this function from the component body. If it can exist statically, it should.

packages/venia-concept/src/components/ProductOptions/option.js

jimbo · 2019-08-07T20:07:01Z

packages/venia-concept/src/components/ProductOptions/option.js

+    ]);
+
+    const valuesMap = useMemo(() => {
+        return new Map(


Good use of useMemo here. 👍

Thanks. Credits to @sirugh. Now I have got understanding of useMemo hook :)

jimbo · 2019-08-07T20:08:13Z

packages/venia-concept/src/components/ProductOptions/option.js

+        selection => {
+            const [selectedValue] = Array.from(selection);
+
+            setSelection(valuesMap.get(selectedValue));


Note that we're now storing the selected label in state, rather than the set of indices.

packages/venia-concept/src/components/ProductOptions/option.js

packages/venia-concept/src/components/ProductOptions/swatch.js

I made changes, need a different reviewer to lookie.

…e-swatch-treatments

jimbo

@sirugh Thanks for taking this one across the finish line. 👍

dani97 · 2019-09-23T18:28:49Z

Thanks @sirugh . I couldn't contribute for over a month. I hope I will join you soon :)

dani97 added 2 commits August 2, 2019 00:49

magento#1390 Swatch Treatments

47fe641

Merge branch 'develop' into feature/1390-update-swatch-treatments

5de2496

dani97 mentioned this pull request Aug 1, 2019

[feature]: Update Swatch Treatments #1390

Closed

PWAStudioBot added the pkg:venia-concept label Aug 1, 2019

dani97 added 2 commits August 2, 2019 17:34

Merge branch 'develop' into feature/1390-update-swatch-treatments

54a42f2

sirugh previously requested changes Aug 2, 2019

View reviewed changes

1390 eslint fix (#3)

d01340d

* fixed eslint issues * prettier and tests corrected

supernova-at assigned jimbo Aug 6, 2019

1390 eslint fix (#4)

8a11af9

options component made as function component and test updated

vercel bot deployed to staging August 7, 2019 14:24 View deployment

PWAStudioBot added pkg:babel-preset-peregrine pkg:graphql-cli-validate-magento-pwa-queries pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. labels Aug 7, 2019

dani97 added 2 commits August 7, 2019 20:00

Merge branch 'develop' of github.com:magento/pwa-studio into feature/…

3d4dcbf

…1390-update-swatch-treatments

Merge branch 'feature/1390-update-swatch-treatments' of github.com:da…

0510e8c

…ni97/pwa-studio into feature/1390-update-swatch-treatments

vercel bot deployed to staging August 7, 2019 14:32 View deployment

dani97 marked this pull request as ready for review August 7, 2019 14:46

Update tests

a329d24

vercel bot deployed to staging August 7, 2019 20:00 View deployment

jimbo suggested changes Aug 7, 2019

View reviewed changes

sirugh removed their assignment Aug 26, 2019

m2-community-project bot removed the Progress: review label Aug 26, 2019

cherdman assigned supernova-at and unassigned supernova-at Sep 18, 2019

cherdman requested a review from supernova-at September 18, 2019 16:33

sirugh added the version: Minor This changeset includes functionality added in a backwards compatible manner. label Sep 18, 2019

awilcoxa added Progress: good first issue Good for newcomers and removed Progress: good first issue Good for newcomers labels Sep 19, 2019

sirugh self-assigned this Sep 20, 2019

Merge remote-tracking branch 'origin/develop' into feature/1390-updat…

2b03d65

…e-swatch-treatments

sirugh force-pushed the feature/1390-update-swatch-treatments branch from ba8d8a1 to 2b03d65 Compare September 23, 2019 14:24

sirugh removed pkg:babel-preset-peregrine pkg:graphql-cli-validate-magento-pwa-queries pkg:peregrine pkg:pwa-buildpack pkg:upward-js Pertains to upward-js reference implementation of UPWARD. pkg:upward-spec Pertains to UPWARD specification package. pkg:venia-concept labels Sep 23, 2019

jimbo approved these changes Sep 23, 2019

View reviewed changes

m2-community-project bot assigned jimbo Sep 23, 2019

m2-community-project bot added the Progress: approved label Sep 23, 2019

m2-community-project bot removed the Progress: approved label Sep 23, 2019

Merge branch 'develop' into feature/1390-update-swatch-treatments

19c398c

dpatil-magento merged commit 9d25740 into magento:develop Sep 23, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Swatch Treatments #1512

Swatch Treatments #1512

dani97 commented Aug 1, 2019 •

edited

Loading

PWAStudioBot commented Aug 1, 2019 •

edited

Loading

sirugh left a comment

vercel bot commented Aug 7, 2019

dani97 commented Aug 7, 2019 •

edited

Loading

jimbo commented Aug 7, 2019

jimbo Aug 7, 2019

jimbo Aug 7, 2019

dani97 Aug 8, 2019

jimbo Aug 7, 2019

jimbo left a comment

dani97 commented Sep 23, 2019

Swatch Treatments #1512

Swatch Treatments #1512

Conversation

dani97 commented Aug 1, 2019 • edited Loading

Description

Related Issue

Verification Steps

Screenshots / Screen Captures (if appropriate)

Proposed Labels for Change Type/Package

Checklist:

PWAStudioBot commented Aug 1, 2019 • edited Loading

sirugh left a comment

Choose a reason for hiding this comment

vercel bot commented Aug 7, 2019

dani97 commented Aug 7, 2019 • edited Loading

jimbo commented Aug 7, 2019

jimbo Aug 7, 2019

Choose a reason for hiding this comment

jimbo Aug 7, 2019

Choose a reason for hiding this comment

dani97 Aug 8, 2019

Choose a reason for hiding this comment

jimbo Aug 7, 2019

Choose a reason for hiding this comment

jimbo left a comment

Choose a reason for hiding this comment

dani97 commented Sep 23, 2019

dani97 commented Aug 1, 2019 •

edited

Loading

PWAStudioBot commented Aug 1, 2019 •

edited

Loading

dani97 commented Aug 7, 2019 •

edited

Loading