feat(recentSearches): add remove button #326

eunjae-lee · 2020-09-24T09:30:14Z

Summary

This PR adds a delete button to recent-searches plugin. It works fine, but when deleting an item, it calls refresh which leads to a whole re-render which causes unnecessary API calls.

I will try to find a way to replace only the items from this plugin without re-triggering other getSources calls.

[Example →]

codesandbox-ci · 2020-09-24T09:31:30Z

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit c00d92e:

Sandbox	Source
algolia/autocomplete.js: js	Configuration
algolia/autocomplete.js: js (forked)	PR

francoischalifour · 2020-09-29T08:52:53Z

It works fine, but when deleting an item, it calls refresh which leads to a whole re-render which causes unnecessary API calls.

It's not too much of a deal because the API calls are cached by the search client (and it's good practice to do so). We can therefore rely on that for now.

eunjae-lee · 2020-09-29T08:59:40Z

It works fine, but when deleting an item, it calls refresh which leads to a whole re-render which causes unnecessary API calls.

It's not too much of a deal because the API calls are cached by the search client (and it's good practice to do so). We can therefore rely on that for now.

I don't think we currently have any cache being used with the search client. We can configure it, but again, if our users have external sources that cannot rely on cache, it's going to cause excessive API calls, unless we provide an autocomplete-level cache layer.

I've managed to implement updateSuggestionItems. What do you think? (I've removed refresh, which can be reverted back if we want)

francoischalifour · 2020-09-29T10:01:27Z

The search client does have cache enabled by default. Notice that it doesn't trigger any network request when the same request already been sent.

We should go for a minimal API surface area to keep this library focused and easy to grasp. We shouldn't let this project become like our previous libraries that went out of control. So I'm in favor of leveraging refresh until there are actual issues with it.

eunjae-lee · 2020-09-29T13:16:01Z

The search client does have cache enabled by default. Notice that it doesn't trigger any network request when the same request already been sent.

We should go for a minimal API surface area to keep this library focused and easy to grasp. We shouldn't let this project become like our previous libraries that went out of control. So I'm in favor of leveraging refresh until there are actual issues with it.

Thanks for the URL. I'll watch it.

I checked again with refresh, and ffacd6e is the one where refresh function was used and this is the example using this version. It missed cache. I looked closely and it was because as "recent searches" items changed (deleted), the following changed:

facetFilters: [...recentSearches.data.getFacetFilters()]

which led to a different search, and the cache miss. I don't know how to fix this. Any idea?

francoischalifour · 2020-09-29T13:25:45Z

Ah right, don't have a proper solution for now, but that's not essential 🙂

eunjae-lee · 2020-09-29T14:13:57Z

Okay then I'll revert to the version using refresh.

eunjae-lee · 2020-09-29T15:20:06Z

packages/autocomplete-plugin-recent-searches/package.json

@@ -15,9 +15,9 @@
  },
  "scripts": {
    "build:clean": "rm -rf ./dist",
-    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm  --ignore '**/*/__tests__/'",
+    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm --ignore '**/*/__tests__/'  && cp src/style.css ./dist/esm/",


I don't know what to do with the style.css file here, so I just copied it to dist on build at the moment.

We'll think about this as a next step. It'd be nice to be able to do:

import '@algolia/autocomplete-plugin-recent-searches/dist/style.css'; // Like DocSearch, with `root/style.js` importing `root/dist/style.css` import '@algolia/autocomplete-plugin-recent-searches/style';

Can you show me where it's done? I will apply it in a separate PR.
So far I've found only this: https://github.com/algolia/docsearch/blob/next/packages/docsearch-react/style/variables.js#L1:L1

It happens there:

https://github.com/algolia/docsearch/blob/56ab94a6b8f445d69f0d63e93de7479a07f8e192/packages/docsearch-react/package.json#L15

https://github.com/algolia/docsearch/blob/56ab94a6b8f445d69f0d63e93de7479a07f8e192/packages/docsearch-react/style/index.js

Hmm. I think I've done it: c00d92e
Did I do it right? What is enabling this? @babel/preset-env?

It looks good. That's the style loader of the bundler used that understands it (webpack, Parcel etc.).

Ah okay. It's the consuming side which understands it.

eunjae-lee · 2020-09-29T15:21:01Z

pinging @francoischalifour for review

francoischalifour · 2020-09-30T12:02:02Z

packages/autocomplete-plugin-recent-searches/package.json

@@ -15,9 +15,9 @@
  },
  "scripts": {
    "build:clean": "rm -rf ./dist",
-    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm  --ignore '**/*/__tests__/'",
+    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm --ignore '**/*/__tests__/'  && cp src/style.css ./dist/esm/",


We'll think about this as a next step. It'd be nice to be able to do:

import '@algolia/autocomplete-plugin-recent-searches/dist/style.css'; // Like DocSearch, with `root/style.js` importing `root/dist/style.css` import '@algolia/autocomplete-plugin-recent-searches/style';

packages/autocomplete-plugin-recent-searches/src/createRecentSearchesPlugin.ts

examples/js/package.json

packages/autocomplete-plugin-recent-searches/src/createRecentSearchesPlugin.ts

francoischalifour · 2020-10-01T09:01:07Z

packages/autocomplete-plugin-recent-searches/src/createRecentSearchesPlugin.ts

-                      <path d="M13 3c-4.97 0-9 4.03-9 9H1l3.89 3.89.07.14L9 12H6c0-3.87 3.13-7 7-7s7 3.13 7 7-3.13 7-7 7c-1.93 0-3.68-.79-4.94-2.06l-1.42 1.42C8.27 19.99 10.51 21 13 21c4.97 0 9-4.03 9-9s-4.03-9-9-9zm-1 5v5l4.28 2.54.72-1.21-3.5-2.08V8H12z"/>
-                    </svg>
+            item({ item, root }) {
+              if (!root.classList.contains('aa-RecentSearchesItem')) {


If you were using a framework like Vue or React, you wouldn't do this but rather create a sub-container in the root. Why mutate the class of the root here then?

You're right. aa-Item was a flex container, so I tried to leverage it. But actually I was uncomfortable doing that.
2da7b94

francoischalifour · 2020-10-01T10:26:56Z

packages/autocomplete-plugin-recent-searches/package.json

@@ -15,9 +15,9 @@
  },
  "scripts": {
    "build:clean": "rm -rf ./dist",
-    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm  --ignore '**/*/__tests__/'",
+    "build:esm": "babel src --root-mode upward --extensions '.ts,.tsx' --out-dir dist/esm --ignore '**/*/__tests__/'  && cp src/style.css ./dist/esm/",


It looks good. That's the style loader of the bundler used that understands it (webpack, Parcel etc.).

eunjae-lee force-pushed the feat/add-remove-button-to-recent-searches branch from 5f9172c to 8de6db3 Compare September 24, 2020 09:59

eunjae-lee marked this pull request as ready for review September 29, 2020 08:59

eunjae-lee requested a review from francoischalifour September 29, 2020 08:59

eunjae-lee force-pushed the feat/add-remove-button-to-recent-searches branch from d15ba39 to a12d790 Compare September 29, 2020 15:09

feat(recentSearches): add remove button

8273bc1

eunjae-lee force-pushed the feat/add-remove-button-to-recent-searches branch from ef1aa4d to 72d2015 Compare September 29, 2020 15:17

eunjae-lee added 2 commits September 29, 2020 17:19

pass refresh to getSources

49a7ca9

update styles

18627e8

eunjae-lee force-pushed the feat/add-remove-button-to-recent-searches branch from 72d2015 to 18627e8 Compare September 29, 2020 15:19

eunjae-lee commented Sep 29, 2020

View reviewed changes

francoischalifour reviewed Sep 30, 2020

View reviewed changes

eunjae-lee added 3 commits September 30, 2020 14:35

rename delete to remove and add title

f4e0875

remove return null

f9ef208

extract icon

63ff715

eunjae-lee requested a review from francoischalifour September 30, 2020 12:42

francoischalifour reviewed Sep 30, 2020

View reviewed changes

examples/js/package.json Outdated Show resolved Hide resolved

packages/autocomplete-plugin-recent-searches/src/createRecentSearchesPlugin.ts Outdated Show resolved Hide resolved

eunjae-lee added 2 commits October 1, 2020 10:40

update styles

2acaf86

bump the version

3e5b77b

eunjae-lee requested a review from francoischalifour October 1, 2020 08:45

francoischalifour reviewed Oct 1, 2020

View reviewed changes

eunjae-lee added 2 commits October 1, 2020 11:24

remove unecessary typing

1ed3312

wrap with a sub-container

2da7b94

export style

c00d92e

francoischalifour approved these changes Oct 1, 2020

View reviewed changes

eunjae-lee merged commit 648f1e8 into next Oct 1, 2020

eunjae-lee deleted the feat/add-remove-button-to-recent-searches branch October 1, 2020 13:35

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(recentSearches): add remove button #326

feat(recentSearches): add remove button #326

eunjae-lee commented Sep 24, 2020 •

edited

Loading

codesandbox-ci bot commented Sep 24, 2020 •

edited

Loading

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 29, 2020

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 29, 2020

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 29, 2020

eunjae-lee Sep 29, 2020

francoischalifour Sep 30, 2020

eunjae-lee Sep 30, 2020

francoischalifour Sep 30, 2020

eunjae-lee Oct 1, 2020

francoischalifour Oct 1, 2020

eunjae-lee Oct 1, 2020

eunjae-lee commented Sep 29, 2020

francoischalifour Sep 30, 2020

francoischalifour Oct 1, 2020

eunjae-lee Oct 1, 2020

francoischalifour Oct 1, 2020

feat(recentSearches): add remove button #326

feat(recentSearches): add remove button #326

Conversation

eunjae-lee commented Sep 24, 2020 • edited Loading

Summary

codesandbox-ci bot commented Sep 24, 2020 • edited Loading

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 29, 2020

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 29, 2020

francoischalifour commented Sep 29, 2020

eunjae-lee commented Sep 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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eunjae-lee commented Sep 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

eunjae-lee commented Sep 24, 2020 •

edited

Loading

codesandbox-ci bot commented Sep 24, 2020 •

edited

Loading