Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[ui] image gallery search bar #1816

Merged
merged 5 commits into from
Nov 30, 2022
Merged

[ui] image gallery search bar #1816

merged 5 commits into from
Nov 30, 2022

Conversation

mugulmd
Copy link
Contributor

@mugulmd mugulmd commented Nov 4, 2022

Description

This PR introduces a new search bar in the image gallery to filter viewpoints using their filename or their uid.

TODO

  • add search bar UI
  • search based on filename
  • search based on uid
  • focus search bar from gallery with tab

@mugulmd mugulmd added the UI label Nov 4, 2022
@mugulmd mugulmd self-assigned this Nov 4, 2022
@mugulmd mugulmd marked this pull request as ready for review November 7, 2022 13:46
Copy link
Contributor

@cbentejac cbentejac left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The filtering works if you're using regular strings, and partially works when using regular expressions (both for the filtering in the Image Gallery and in the metadata display, which is not directly concerned by this PR):

  • the "." character works as any character and doesn't seem to generate any errors
  • the "*" characters automatically cancels any filtering that it precedes/follows: as soon as it appears in the filtering string, no matter its position, it's like no filtering is applied. It also generates a syntax error warning in meshroom/ui/qml/Utils/SortFilterDelegateModel.qml at line 95 ("SyntaxError: Invalid regular expression").

I haven't tested any complex regex, and this behaviour seems to be the regular one for the metadata filtering (independently from this PR).

I don't know if we want to support regexes or not in the filtering but in any case, something should probably be done to either support them correctly, or not support them at all.

Apart from that, the code looks good to me.

@mugulmd
Copy link
Contributor Author

mugulmd commented Nov 8, 2022

The filtering works if you're using regular strings, and partially works when using regular expressions (both for the filtering in the Image Gallery and in the metadata display, which is not directly concerned by this PR):

* the "." character works as any character and doesn't seem to generate any errors

* the "*" characters automatically cancels any filtering that it precedes/follows: as soon as it appears in the filtering string, no matter its position, it's like no filtering is applied. It also generates a syntax error warning in meshroom/ui/qml/Utils/SortFilterDelegateModel.qml at line 95 ("SyntaxError: Invalid regular expression").

I haven't tested any complex regex, and this behaviour seems to be the regular one for the metadata filtering (independently from this PR).

I don't know if we want to support regexes or not in the filtering but in any case, something should probably be done to either support them correctly, or not support them at all.

Apart from that, the code looks good to me.

Since we can't expect users to know about how regexp work, it would feel like a bug to them when entering characters like "." as it would trigger quite a lot of matches, so I removed the support for regexp: we're simply trying to match a sub-string now (which is coherent with how things are done in the new node menu of the graph editor by the way).

@fabiencastan fabiencastan added this to the Meshroom 2022.1.0 milestone Nov 30, 2022
@fabiencastan
Copy link
Member

Maybe one day it would be good to support simple expressions like
https://docs.python.org/3/library/fnmatch.html

@fabiencastan fabiencastan merged commit 02b2d84 into develop Nov 30, 2022
@fabiencastan fabiencastan deleted the mug/searchImgGallery branch November 30, 2022 11:08
@mugulmd
Copy link
Contributor Author

mugulmd commented Nov 30, 2022

Maybe one day it would be good to support simple expressions like https://docs.python.org/3/library/fnmatch.html

Added to my TODO list 😄

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants