create products layout. #3

sanjithkumar017 · 2020-05-29T03:31:03Z

No description provided.

Anupama-Unbxd · 2020-05-29T05:15:04Z

src/App.js


+        this.state = {
+            unbxdCore:


why is this object required to be on state? Can we keep it as an instance variable?

So that the changes are passed on to children.

Anupama-Unbxd · 2020-05-29T05:16:06Z

src/App.js

+        this.state = {
+            unbxdCore:
+                new UnbxdSearch({ ...searchConfigurations, siteName, siteKey, callBackFn: this.unbxdCallBack }),
+            helpers: this.helpers


same here, is this a dynamically changing value to be kept on state? can we just use instance variable instead?

Yes, it is dynamic.

Anupama-Unbxd · 2020-05-29T05:20:24Z

src/modules/Products/GridView.js

+        variantMap,
+        unbxdProductCardClickHandler } = props;
+
+    return (<div className={`Unbx-product-container Unbx-grid-view grid grid-cols-${per_row} gap-4 p-4`}>{


Lets keep the prefix as UNX-. Will these other classes end up in the final rendered HTML?
These classnames look very generic & hence might clash with the CSS rules already defined by the customer

Anupama-Unbxd · 2020-05-29T05:32:06Z

src/modules/Products/GridView.js

+                variantMap={variantMap}
+                per_row={per_row}
+                key={product.uniqueId}
+                unbxdProductCardClickHandler={unbxdProductCardClickHandler} />)


can we move the click to the parent so that we have only 1 click handler instead of one per product (if we show 100 products on page, we will have 100 click handlers)

Anupama-Unbxd · 2020-05-29T07:08:43Z

src/modules/Products/GridView.js

+
+GridView.propTypes = {
+    products: PropTypes.arrayOf(PropTypes.object).isRequired,
+    per_row: PropTypes.number.isRequired,


lets have a standard of camelCase props everywhere

Anupama-Unbxd · 2020-05-29T07:10:40Z

src/modules/Products/GridView.js

+    per_row: PropTypes.number.isRequired,
+    fieldMap: PropTypes.object.isRequired,
+    variantMap: PropTypes.object.isRequired,
+    unbxdProductCardClickHandler: PropTypes.func.isRequired


Lets keep naming standard for click handlers as per the React standards in the format on<Event>.
For example in this case, it would be onProductClick.

Anupama-Unbxd · 2020-05-29T07:11:28Z

src/modules/Products/ListView.js

+
+const ListView = ({ products = [], fieldMap, isVariant, variantMap, unbxdProductCardClickHandler }) => {
+
+    return (<div className={`Unbx-product-container Unbx-grid-view grid grid-cols-1 gap-4 p-4`}>{


same as above, lets add prefix as UNX- and try to remove the non-prefixed class names

Anupama-Unbxd · 2020-05-29T07:12:00Z

src/modules/Products/ListView.js

+    fieldMap: PropTypes.object.isRequired,
+    isVariant: PropTypes.bool.isRequired,
+    variantMap: PropTypes.object.isRequired,
+    unbxdProductCardClickHandler: PropTypes.func.isRequired


same as the GridView props comments above

Anupama-Unbxd · 2020-05-29T07:12:14Z

src/modules/Products/NoProducts.js

+import React from 'react';
+
+const NoProducts = () => {
+    return (<div className='Unbx-no-products-container'>Sorry! No products found!</div>)


prefix as UNX-

Anupama-Unbxd · 2020-05-29T07:16:57Z

src/modules/Products/ProductCards/GridProductCard.js

+const GridProductCard = ({ product, fieldMap, variantMap, unbxdProductCardClickHandler }) => {
+
+    //Get the datas from the product bases on fieldMap and create the card
+    const productValues = productFieldsMapper(product, fieldMap, variantMap);


name the functions with the "action" type present in the name, like mapProductFields or getProductFields or setProductFields. In general, names would mostly be of the form <action><entity> , where action could be get, set, process, map etc.

Anupama-Unbxd · 2020-05-29T07:18:14Z

src/modules/Products/ProductCards/GridProductCard.js

+            <img className='Unbx-product-card Unbx-image' src={imageUrl[0]} />
+            <p className='Unbx-product-card Unbx-product-name'>{productName}</p>
+            <p className='Unbx-product-card Unbx-price'>{price}</p>
+            <p className='Unbx-product-card Unbx-selling-price'>{sellingPrice}</p>


as the parent has the -product-card class already, do we need this in all the children elements?

Anupama-Unbxd · 2020-05-29T07:18:42Z

src/modules/Products/ProductCards/ListProductCard.js

+            <img className='Unbx-product-card Unbx-image' src={imageUrl[0]} />
+            <p className='Unbx-product-card Unbx-product-name'>{productName}</p>
+            <p className='Unbx-product-card Unbx-price'>{price}</p>
+            <p className='Unbx-product-card Unbx-selling-price'>{sellingPrice}</p>


as the parent has the -product-card class already, do we need this in all the children elements?

Anupama-Unbxd · 2020-05-29T07:19:13Z

src/modules/Products/ProductCards/ListProductCard.js

+
+    //Add support for router as a config
+    return (<div className='Unbx-product-card-container'>
+        <a href={product.productUrl} className={`Unbx-product-card Unbx-grid-card`} onClick={unbxdProductCardClickHandler}>


it should be list-card here right instead of grid-card ?

Anupama-Unbxd · 2020-05-29T07:50:32Z

src/modules/Products/index.js

@@ -0,0 +1,143 @@
+import React from 'react';


Lets use index.js files to export all files of a folder. Rename this file as Products.js

Anupama-Unbxd · 2020-05-29T07:51:24Z

src/modules/Products/context/index.js

+const ProductContextConsumer = ProductContext.Consumer;
+
+export { ProductContextProvider, ProductContextConsumer }
+export default ProductContext;


Lets use index.js files to export all files of a folder. Rename this file as ProductContext.js

Anupama-Unbxd · 2020-05-29T07:53:13Z

src/modules/Products/index.js

@@ -0,0 +1,143 @@
+import React from 'react';
+import AppContext, { AppContextConsumer } from '../../common/context'
+import { ProductContextProvider } from './Context'


shouldn't this be ./context with smallcase "c"

Anupama-Unbxd · 2020-05-29T07:54:35Z

src/modules/Products/index.js

+
+Products.propTypes = {
+    per_row: PropTypes.number,
+    per_page: PropTypes.number,


Use camelCase standard for prop names

Anupama-Unbxd · 2020-05-29T07:55:14Z

src/modules/Products/index.js

+    per_row: PropTypes.number,
+    per_page: PropTypes.number,
+    fieldMap: PropTypes.object.isRequired,
+    ZeroResultsTemplate: PropTypes.oneOfType([PropTypes.element, PropTypes.bool, PropTypes.func]),


As this is a React component & not a template name it as, "ZeroResultsComponent".
Also, how would bool be one of the accepted types?

Anupama-Unbxd · 2020-05-29T07:56:12Z

src/modules/Products/index.js

+    fieldMap: PropTypes.object.isRequired,
+    ZeroResultsTemplate: PropTypes.oneOfType([PropTypes.element, PropTypes.bool, PropTypes.func]),
+    infiniteScroll: PropTypes.bool,
+    loadOnClick: PropTypes.bool,


Please refer to the configs sheet. Lets accept both these settings as paginationType

Anupama-Unbxd · 2020-05-29T07:56:54Z

src/modules/Products/index.js

+    ZeroResultsTemplate: PropTypes.oneOfType([PropTypes.element, PropTypes.bool, PropTypes.func]),
+    infiniteScroll: PropTypes.bool,
+    loadOnClick: PropTypes.bool,
+    variants: PropTypes.bool,


Lets rename it to "showVariants" as mentioned in the configs sheet

Anupama-Unbxd · 2020-05-29T07:59:18Z

src/modules/Products/utils/index.js

@@ -0,0 +1,24 @@
+const productFieldsMapper = (product, fieldMap, variantMap) => {


Rename this file to utils.js. The other possible files under this folder could be Constants.js for enums/constants etc. Then we can use index.js to export both utils.js & Constants.js exports

Anupama-Unbxd · 2020-05-29T08:00:40Z

src/modules/Products/ProductCards/GridProductCard.js

@@ -0,0 +1,37 @@
+import React from 'react';


Lets keep small case beginning names for folder names. Here it would be src/modules/products/productCards/GridProductCard.js
Only component class file names should begin with a capital letter.

Anupama-Unbxd · 2020-05-29T08:07:53Z

src/modules/Products/index.js

+    static contextType = AppContext;
+
+    static ViewTypes = ViewTypes;
+    static ProductsView = ProductsView;


I don't think any of these are required right? because they are all imported directly?

This is was done with the intention of providing flexibility to the end user. This component can be used in following way:

Anupama-Unbxd · 2020-05-29T08:09:09Z

src/modules/Products/index.js

+            infiniteScroll, loadOnClick, fieldMap, variantMap } = this.props
+
+        this.state = {
+            isGrid: true,


name this variable as "viewType" instead with enum values like GRID , LIST etc, this will give us flexibility to add more view types in future if needed

Anupama-Unbxd · 2020-05-29T08:10:42Z

src/modules/Products/index.js

+            per_row,
+            per_page,
+            infiniteScroll,
+            loadOnClick,


merge both these configs in paginationType with enum values INFINITE_SCROLL, CLICK_N_SCROLL or FIXED_PAGINATION

Anupama-Unbxd · 2020-05-29T08:10:57Z

src/modules/Products/index.js

+
+        this.state = {
+            isGrid: true,
+            onViewToggle: this.onViewToggle,


why is this required to be on the state?

…croll.

latest

create products layout. swatches is pending.

e3adff8

Anupama-Unbxd reviewed May 29, 2020

View reviewed changes

sanjithkumar017 added 2 commits May 30, 2020 17:02

rename file paths

bba7480

resolved issues.

40711c1

sanjithkumar017 force-pushed the products_layout branch from 2dc1a77 to 40711c1 Compare May 30, 2020 11:33

sanjithkumar017 requested a review from Anupama-Unbxd May 30, 2020 11:33

sanjithkumar017 force-pushed the products_layout branch from 970970d to 080ea29 Compare June 1, 2020 08:45

files names refactor. set pagesize only on infinite scroll or clickns…

f38af4f

…croll.

sanjithkumar017 force-pushed the products_layout branch from 080ea29 to f38af4f Compare June 1, 2020 09:10

organised passing of data and helpers.

32c0ec0

sanjithkumar017 force-pushed the products_layout branch from f03d6a1 to 32c0ec0 Compare June 1, 2020 18:25

sanjithkumar017 added 3 commits June 2, 2020 00:12

Fix folder name problem (wip)

dfafd5c

Fixed folder name problem

66b0912

adding action track

8049f97

Anupama-Unbxd merged commit 593356d into unbxd:master Jun 8, 2020

sanjithkumar017 changed the title ~~create products layout. swatches is pending.~~ create products layout. Jun 19, 2020

sanjithkumar017 deleted the products_layout branch July 3, 2020 07:25

Anupama-Unbxd pushed a commit that referenced this pull request Mar 24, 2022

Merge pull request #3 from unbxd/master

5e8c9d2

latest

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

create products layout. #3

create products layout. #3

sanjithkumar017 commented May 29, 2020

Anupama-Unbxd May 29, 2020

sanjithkumar017 May 30, 2020

Anupama-Unbxd May 29, 2020

sanjithkumar017 May 30, 2020

Anupama-Unbxd May 29, 2020 •

edited

Loading

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020 •

edited

Loading

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

sanjithkumar017 May 30, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020

Anupama-Unbxd May 29, 2020


		const ListView = ({ products = [], fieldMap, isVariant, variantMap, unbxdProductCardClickHandler }) => {

		return (<div className={`Unbx-product-container Unbx-grid-view grid grid-cols-1 gap-4 p-4`}>{

		@@ -0,0 +1,24 @@
		const productFieldsMapper = (product, fieldMap, variantMap) => {

create products layout. #3

create products layout. #3

Conversation

sanjithkumar017 commented May 29, 2020

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

Anupama-Unbxd May 29, 2020 • 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

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

Anupama-Unbxd May 29, 2020 • 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

Anupama-Unbxd May 29, 2020 •

edited

Loading

Anupama-Unbxd May 29, 2020 •

edited

Loading