mgr/dashboard: Improve table search #20807
Conversation
Devp00l
changed the title
Devp00l
force-pushed
the
improve-table-search
branch
from
1d55953
to
8636994
Compare
Devp00l
changed the title
|
@Devp00l this is a general comment: if the frontend is now written in typescript why don't I see type declarations in the code? I know that most types can be inferred, and probably typescript compiler will do that, but it might also be the case that the compiler cannot infer the type and fallbacks to the |
|
@rjfd I will add missing declarations |
Devp00l
force-pushed
the
improve-table-search
branch
from
8636994
to
23c7284
Compare
|
@rjfd Added the missing declarations |
|
Yes, the various pages and datatable features need to be documented. Right now, this is still TBD, but a "Dashboard users guide" would be nice to have. |
|
Wouldn't it be better to separate the search strings only by |
|
@tspmelo if you search any search function on a page with spaces in between you won't search multiple words at once, only if you hit ctrl+f you will search for the exact pattern, but this is another type of search. This search works as you would expect a page search to function. |
There was a problem hiding this comment.
ATM we can't filter for columns with multiple words.
If we search for "public address:192" no result will be found because it was searching for "public" and "address:192".
I would suggest we force the comma separator if there is a reference to any column. Otherwise we can still use the space as a separator.
|
@tspmelo You don't need to type the full column - it's enough to type |
Devp00l
force-pushed
the
improve-table-search
branch
3 times, most recently
from
df52079
to
4af511f
Compare
LenzGr
changed the title
|
Using |
|
jenkins retest this please |
|
@Devp00l "make check" failed because of linting problems in the frontend code, could you fix it? |
Devp00l
force-pushed
the
improve-table-search
branch
from
4af511f
to
6a09b54
Compare
| if (searchWords.length === 2) { | ||
| tempColumns = [...columns]; | ||
| columns = columns.filter((c) => c.name.toLowerCase().indexOf(searchWords[0]) !== -1); | ||
| const searchTerms: string[] = currentSearch.pop().split(':'); |
There was a problem hiding this comment.
you could replace + with , after pop(). This way you don't need to define getSearchTerm.
| }); | ||
| const searchTerm: string = getSearchTerm(_.last(searchTerms)); | ||
| data = this.basicDataSearch(searchTerm, data, columns); | ||
| // Checks if user searches for column but he is still typing |
There was a problem hiding this comment.
I think the search should focus only on the values of each field, it should only check the column if there is a : after the string.
You could delay the search until the user finished typing, that way you would never get a empty result caused by an incomplete search.
There was a problem hiding this comment.
This way you also won't get an empty list, and it's a nicer user experience to provide instant search results.
| return; | ||
| } | ||
| if (_.isArray(cellValue)) { | ||
| cellValue = cellValue.join(''); |
There was a problem hiding this comment.
We should use the same char that we normally use to display the tables.
I will list some tests I made on the OSD page where all rows have "up, in" as the Status.
up, in-> Didn't return anything. Maybe this should return all rows, but I'm ok with this.up in-> Returned all rows. Correct."up, in"-> Didn't return anything. Should return all rows, because I typed exactly what I see in the rows."up in"-> Didn't return anything. Correct.status:up status:in-> Returned all rows. Correct.pi-> Return all rows. Should have returned empty.
There was a problem hiding this comment.
Commas are ignored now.
I couldn't reproduce the pi example it will return nothing for me.
Now all other examples will show all up and in rows.
It's OK if you miss the comma in quotes as you are not searching any string, you are searching an array. Of course ignoring the comma will loose the way to find strings with a comma in name but those strings a pretty rare, comma is mostly used as separator for items.
Devp00l
force-pushed
the
improve-table-search
branch
from
6a09b54
to
8a97df3
Compare
| }).length > 0 | ||
| ); | ||
| }); | ||
| console.log(search); |
Devp00l
force-pushed
the
improve-table-search
branch
from
8a97df3
to
36372ab
Compare
| } | ||
| return data.filter(d => { | ||
| return columns.filter(c => { | ||
| let cellValue: any = _.get(d, c.prop); |
There was a problem hiding this comment.
If the column is using a pipe, the first transformation that you should do is applying that pipe. Otherwise user will not be filtering by what he is seeing.
if (!_.isUndefined(c.pipe)) {
cellValue = c.pipe.transform(cellValue);
}
There was a problem hiding this comment.
Nice spotting :) Implemented it 👍
You can now search for multiple words at a time if you separate them by comma or space. The other improvement is that you can specify which column should be searched by a search term. For example if you wand to filter for a row with the ID 3 you type "id:3". The column name is case insensitive and is separated from the search word by a colon. Signed-off-by: Stephan Müller <smueller@suse.com>
Removed the usage of using comma to indicated a new search term and introduced the possibility to use quoted strings in order to search for spaces too. Also improved the column search handling in order to not get no search results found while typing a column name. Signed-off-by: Stephan Müller <smueller@suse.com>
Devp00l
force-pushed
the
improve-table-search
branch
from
36372ab
to
ac88559
Compare
|
jenkins retest this please |
1 similar comment
|
jenkins retest this please |
You can now search for multiple words at a time if you separate them by
comma or space. The other improvement is that you can specify which
column should be searched by a search term. For example if you wand to
filter for a row with the ID 3 you type "id:3". The column name is case
insensitive and is separated from the search word by a colon.
Signed-off-by: Stephan Müller smueller@suse.com