algolia / algolia-pwa-demo

3 stars 1 forks source link

Instant Search functionality & Autocomplete refactoring #5

Closed htuzel closed 5 months ago

htuzel commented 5 months ago

What is New?

You may ignore the autocomplete folder, there are only style fixes and tiny bug fix and refactorings

htuzel commented 5 months ago

Big work 💪

Implemented instantsearch functionality

Could you detail more what has been done to help the review? For example, I see in the overrides/app/pages/product-list/partials/ directory some algolia-xxx.jsx file, which have a Salesforce copyright (Copyright (c) 2023, Salesforce, Inc.). It looks like those files are coming from us. Did you copy them from an existing implementation, or wrote them entirely? If it's coming from us, the copyright should reflect that.

Could we maybe move them in a dedicated directory? The goal of this demo is to help users identify what files they need to copy to implement Algolia on their existing website so it will be nice to separate the Algolia added files.

Hello Sylvain,

I copied them from https://github.com/algolia/pwa-kit but also made some changes.. Not sure if we need to keep copyright or not but also many of them is from initial pwa implementation.. Moving them to a folder definitely make sense, will handle it

sbellone commented 5 months ago

Ok now I see that you've moved everything in a different directory. I was only referring about the Algolia-specific code (the algolia-xxx.jsx files).

The way I see it, is to keep in overrides/app/pages/product-list/partials/ the files that are copied directly from the PWA Kit demo (and very slightly modified): https://github.com/SalesforceCommerceCloud/pwa-kit/tree/eb5183b7f337665e4a4a16b44972fa9d8470c3d9/packages/template-retail-react-app/app/pages/product-list/partials

And put the pure Algolia implementation in a specific directory. For example, having all algolia-xxx.jsx files in a overrides/app/pages/product-list/algolia/ directory or similar.

htuzel commented 5 months ago

Ok now I see that you've moved everything in a different directory. I was only referring about the Algolia-specific code (the algolia-xxx.jsx files).

The way I see it, is to keep in overrides/app/pages/product-list/partials/ the files that are copied directly from the PWA Kit demo (and very slightly modified): https://github.com/SalesforceCommerceCloud/pwa-kit/tree/eb5183b7f337665e4a4a16b44972fa9d8470c3d9/packages/template-retail-react-app/app/pages/product-list/partials

And put the pure Algolia implementation in a specific directory. For example, having all algolia-xxx.jsx files in a overrides/app/pages/product-list/algolia/ directory or similar.

Hi @sbellone

I worked on this topic a bit. I understand your point about organizing the Algolia-specific files separately. However, since everything in the overrides directory is already considered new and modified, I believe keeping the Algolia files there should be sufficient. Because almost any previous users wont be aware of previous Salesforce-Algolia PWA Repository but PWA-Kit

Users will inherently understand that these files are different from the original PWA Kit repo.

Let me know if you have any concerns or further thoughts on this.

Best,