thombruce / toodles

✅ A super simple todo app
https://toodles.thombruce.com/
GNU General Public License v3.0
0 stars 0 forks source link

Add search #81

Closed thombruce closed 1 year ago

thombruce commented 1 year ago

closes #76

thombruce commented 1 year ago

Presently if I search for dog+dinosaur, it matches either term. Would it be preferable to try and match ALL terms instead?

github-actions[bot] commented 1 year ago

Coverage Summary for `./packages/web`

Status Category Percentage Covered / Total
🟢 Lines 69.18% / 60% 348 / 503
🟢 Statements 69.18% / 60% 348 / 503
🟢 Functions 60% / 60% 27 / 45
🟢 Branches 70.68% / 60% 41 / 58
File Coverage
File Stmts % Branch % Funcs % Lines Uncovered Lines
packages/web/src/components/ContextTag.vue 20% 100% 0% 20% 4-15
packages/web/src/components/MultiBar.vue 100% 100% 25% 100%
packages/web/src/components/ProgressBar.vue 70.58% 75% 100% 70.58% 8-10, 13-19
packages/web/src/components/ProjectTag.vue 20% 100% 0% 20% 4-15
packages/web/src/components/TagTag.vue 71.42% 100% 0% 71.42% 6-7
packages/web/src/components/TodoItem.vue 72.54% 40% 28.57% 72.54% 20-24, 26-27, 31-37
packages/web/src/components/TodoList.vue 75% 100% 100% 75% 12-16
packages/web/src/components/TodoPriority.vue 20% 100% 0% 20% 4-15
packages/web/src/components/TodoText.vue 65.85% 58.33% 83.33% 65.85% 15-18, 21-24, 27-28, 33-36
packages/web/src/models/Todo.ts 89.61% 75% 100% 89.61% 27-32, 46-47
packages/web/src/plugins/dexie.ts 70.96% 100% 100% 70.96% 21-22, 25-31
packages/web/src/plugins/lunr.ts 78.57% 100% 0% 78.57% 10-12
packages/web/src/plugins/timepiece.ts 71.42% 60% 50% 71.42% 14-19
packages/web/src/stores/todos.ts 66.19% 90% 71.42% 66.19% 13-33, 46-48, 52-54, 58-60, 64-66, 70-72, 76-78, 84, 88-95
thombruce commented 1 year ago

I really want it to be a dual add+search input too. And as the user adds more words, the likelihood increases they'll add a word that causes a search result to show up, whereas if the search grew more and more specific instead of more vague, that could be a clear way to differentiate between it being a search vs add input.

thombruce commented 1 year ago

Just tested adding unique UNIQUE unIque and all three are added to search index values but are identical: uniqu. If we were storing/searching the full index, this might be beneficial, but we're not - we're storing individual words only, so the dups are redundant.

thombruce commented 1 year ago

Still need to test the responses to:

lunr.tokenizer.separator can be modified if that helps the cause.

EDIT: This is simple but it works a treat - https://github.com/thombruce/toodles/pull/81/commits/3c24ecea20a0a9b8b87fa7e93b6aea4806f16402

thombruce commented 1 year ago

Easiest way to implement search+input shared is maybe...

We can also handle search on Ctrl+Enter, if still desirable...

And with that basic form implemented, we can try expanding on the behaviour to intelligently infer the kind of event, but that's good groundwork.

thombruce commented 1 year ago

Mostly done, but behaviour is inconsistent. If I go to a project page for instance and search for a value known to exist elsewhere, list is empty. This differs from the main list where list just shows main list if the results set is empty...

This can be solved a couple of ways...

  1. Store query in store, so if query is present THEN use the results list.
  2. ...

Well, for two I was going to suggest implementing multi-word search as is desired would fix this... it wouldn't though. Still desirable, but I think we want to determine list to use by presence of query?


It somewhat doesn't feel right that the whole list disappears when querying. When this is feature-complete, we need to address the feel of it and modify to preference.

thombruce commented 1 year ago

Seem to have broken marking todos as done. Complaint in console refers to Dexie's inability to clone. Seen before, should be an easy fix; will look at this before marking ready.

Probably has to do with tokens index. What do we think? At present I'm pretty sure I just hand over the entire todo to be reinserted; Dexie's update hook will look at this and try to reparse tokens...

thombruce commented 1 year ago

Fixed.

This does about everything I want it to now. Just a question of whether the feel is right, which is a matter of playing with it for a bit. I'll update with ideas if I have any but essentially we're just trying to decide between...

  1. Current behaviour: If no search matches, full list is shown
  2. Explicit search behaviour: If no search matches, empty list is shown

We could also say something like... hey, non-matches are grayed-out (can easily use opacity to accomplish this). That might be worth toying with too.

thombruce commented 1 year ago

NOTE: We crucially still need to fix the behaviour on sub-pages like project pages, where if I enter a search term for something that does exist but not for the project... the list is empty.

We can fix this by modifying the getter. If query is empty, fallback on default behaviour.

thombruce commented 1 year ago

Showing a grayed-out list when search returns nothing is a harder problem than anticipated, and I want to handle it as a separate PR. So... shelving that, is there anything else to do here?

I could try to decide on default behaviour, since it remains inconsistent for now... but I think I'd be treading on my own toes looking forward to that future PR. So probably shelving that for now too.

In which case, it remains only a question of... is search itself done? Is it implemented correctly? Is indexing correct?

Search is good, certainly... so what about the index? Right now we save tokens rather than full words, and we search using those tokens too. This accomplishes a sort of fuzzy search. I think clearly we always want to tokenize the query terms, since this is fundamentally how that works. And I think storing these tokenized terms makes most sense too, since they are fundamentally shorter and less to store.

I guess double-check that we're storing them uniquely... would be useful to ditch common terms like 'the' and 'a', that aren't really useful... but I don't know how important this is. It's also... English-specific, and we're not wanting to throw in i18n concerns here yet.

So... I think we're good.