Dataset name text filtering #485
Conversation
matthewma7
commented
Jan 30, 2018
matthewma7
commented
matthewma7
changed the base branch from
master
to
draw_boundary_dataset_and_boundary_filtering
matthewma7
changed the base branch from
draw_boundary_dataset_and_boundary_filtering
to
master
web_external/views/body/DataPanel.js
Outdated
| var regex = new RegExp(keyword, 'i'); | ||
| match = !!regex.exec(dataset.get('name')); | ||
| } catch (ex) { | ||
| match = dataset.get('name').toLocaleLowerCase().indexOf(keyword) !== -1; |
There was a problem hiding this comment.
I recommend moving this below of the catch as match |= dataset.get('name').toLocaleLowerCase().indexOf(keyword) !== -1;. Otherwise, if I have a dataset called "foo.bar" and search for "foo.b", it won't be found (because as a regex that doesn't match), which would be surprising to a user.
There was a problem hiding this comment.
To be clearer:
var match = false;
try {
var regex = new RegExp(keyword, 'i');
match = !!regex.exec(dataset.get('name'));
} catch (ex) {}
match |= dataset.get('name').toLocaleLowerCase().indexOf(keyword) !== -1;
if (match) {
ids.push(dataset.get('_id'));
}
return ids;
|
@matthewma7 @manthey as discussed in the meeting, we would like to extend it to search through the metadata as well. Are we planning on doing that in this branch or another one? |
|
@aashish24 I personally is thinking to do it in another PR because we kind want this to release soon, and I also have a concern that searching meta may end up with too many records, so some mechanism to indicate might be needed (a checkbox, a sorted result, etc) |
Sounds good. We should start tagging releases (thoughts?) |
|
@matthewma7 @manthey how do we enable coverage reporting on PR's? |
|
Girder changed where coverage files are stored. I just made PR #487 to fix the reporting. |
web_external/views/map/MapPanel.js
Outdated
| @@ -336,6 +336,7 @@ const MapPanel = Panel.extend({ | |||
| layer.removeAllAnnotations(); | |||
| } | |||
| }); | |||
| prompt.find('.bootbox-body input').attr('maxlength', '30'); | |||
There was a problem hiding this comment.
Out of curiosity, why 30? I know wiredTiger supports up to 1023 bytes "including structural overhead"; probably half that would be pretty safe.
There was a problem hiding this comment.
I might be wrong. I was thinking other characters from other language takes more bytes than English letters, so I choose it to make it safe. But if 30 is too limiting, I think we change it to a bigger number. (A failed creating boundary is not that devastating)
There was a problem hiding this comment.
Sure -- I think wiredTiger uses UTF-8, which would mean that in theory a codepoint could take 6 bytes, but in practice 4 is sufficient. How about we make it 250? It is large enough most people shouldn't have an issue and small enough Mongo shouldn't have an issue. I only discovered this because I intentionally paste absurd data into forms.
There was a problem hiding this comment.
Sounds good. New commit pushed.
|
@manthey Thank you for the review all the edge case you found. |
|
👍 |
