change search ux #600
change search ux #600
Conversation
|
@sosukesuzuki this looks awesome. |
|
Awesome! |
browser/main/NoteList/index.js
Outdated
| @@ -349,6 +353,71 @@ class NoteList extends React.Component { | |||
| }) | |||
| } | |||
|
|
|||
| getSearchNotes () { | |||
There was a problem hiding this comment.
To begin with my review, I'd like you to move this method to a module such as lib/search or something. But it's ok, I'll start my review :) You can move it after my review.
There was a problem hiding this comment.
I assume searchFromNotes() is more appropriate.
There was a problem hiding this comment.
Thank you for commenting! I think so, too. I'll change it.
browser/main/NoteList/index.js
Outdated
| }) | ||
| }) | ||
| } else if (block.match(/^!.+/)) { | ||
| let block = block.match(/^!(.+)/)[1] |
There was a problem hiding this comment.
This line contains a bug even in production 😨
There was a problem hiding this comment.
You can delete this condition too.
browser/main/NoteList/index.js
Outdated
| return false | ||
| }) | ||
| } | ||
| }) |
browser/main/NoteList/index.js
Outdated
| if (search.trim().length === 0) return [] | ||
| let searchBlocks = search.split(' ') | ||
| searchBlocks.forEach((block) => { | ||
| if (block.match(/^!#.+/)) { |
There was a problem hiding this comment.
You can delete these condition starting from /^!.+/.
browser/main/NoteList/index.js
Outdated
| }) | ||
| }) | ||
| } else if (block.match(/^!.+/)) { | ||
| let block = block.match(/^!(.+)/)[1] |
There was a problem hiding this comment.
You can delete this condition too.
browser/main/NoteList/index.js
Outdated
| } | ||
| return false | ||
| }) | ||
| } else if (block.match(/^#.+/)) { |
There was a problem hiding this comment.
You should make a new method named such as findByTag().
browser/main/NoteList/index.js
Outdated
| return _tag.match(regExp) | ||
| }) | ||
| }) | ||
| } else { |
There was a problem hiding this comment.
Also, you can make a new method such ad findByWord().
browser/main/NoteList/index.js
Outdated
| } else if (note.type === 'MARKDOWN_NOTE') { | ||
| return note.content.match(regExp) | ||
| } | ||
| return false |
There was a problem hiding this comment.
I assume this below is ideal form.
if (isTag(block)) {
const tag = block.match(/#(.+)/)[1]
notes = findByTag(notes, tag)
} else {
notes = findByWord(notes, block)
}
There was a problem hiding this comment.
And also, we have to contemplate to decide these specifications.
There was a problem hiding this comment.
the magic number is not good 🙅
There was a problem hiding this comment.
Thank you for commenting! That is so simple and so good. I'll change to it.
browser/main/NoteList/index.js
Outdated
| @@ -349,6 +353,71 @@ class NoteList extends React.Component { | |||
| }) | |||
| } | |||
|
|
|||
| getSearchNotes () { | |||
| let { data } = this.props | |||
| let search = document.getElementsByClassName('TopBar__control-search-input___browser-main-TopBar-')[0].childNodes[0].value | |||
There was a problem hiding this comment.
The class name depends on webpack, so you have to change it. If I were you, I name the element such as searchInput.
There was a problem hiding this comment.
Thank you for telling it me! I'll add className 'searchInput' to input in Topbar/index.js.
| } | ||
| }) | ||
| return notes | ||
| } |
There was a problem hiding this comment.
Thank you for changing them into the module 😄
And probably, I made a mistake 😨. I assume arranged interface is better (regarding findByTag() and findByWord). So, you should change findByTag(). It's better that to cut out the tag from a block inside the method.
searchBlocks.forEach((block) => {
if (block.match(/^#.+/)) {
notes = findByTag(notes, block)
} else {
notes = findByWord(notes, block)
}
})
And also, you can make a method such as findByTagOrWord(block) to combine the methods for finding. (I'm not sure the argument name (block) is better or not 🤒)
There was a problem hiding this comment.
Thank you for commenting! Certainly it is simple and easy to understand.
|
🎉🎉🎉🎉🎉🎉🎉 |
Before
Display the searched note in a popup window.
After
Display the searched notes in the normal note list.