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

Brand new search bar active layer feature matching functionality + UI/UX improvements #4546

Merged
merged 11 commits into from
Sep 8, 2023

Conversation

nirvn
Copy link
Member

@nirvn nirvn commented Sep 1, 2023

This PR adds a new functionality to the search bar: an active layer feature matching. To begin with, here's a screencast showcasing the UI/UX:

Screencast.from.2023-09-01.11-07-04.webm

Few notes:

  • The search functionality must be activated via its f prefix as the search can be quite costly;
  • Contrary to the all features (within all layers) search, this search functionality allows for single character searches (thanks to it being prefixed), and will not only try to match against a feature's display name but also all text and numerical fields;
  • When an attribute match occurs, the attribute value and attribute name are highlighted, while the feature display name is appended as a second text row (seen in screencast above);
  • It is possible to restrict to search to one specific field by using the following syntax: 'f @ATTRIBUTE_NAME search term'). This matches the behavior found in QGIS and can be super useful in narrowing searches on the spot.

The screencast also shows a nice UX improvements added here. Since we are now having quite a few search functionalities, with a few activated only via prefix, it has become slightly harder for QField users to be made aware of all of the cool stuff the search bar those. To remedy to that, the search bar now shows a list of functionalities when users activate the search bar prior to entering text.

Part of the code logic was taken from @nyalldawson 's equivalent locator filter in QGIS (https://github.com/qgis/QGIS/blob/master/src/app/locator/qgsactivelayerfeatureslocatorfilter.cpp). Insuring that QField and QGIS have matching behaviors was important. I'm pretty sure we'll get a few QField users aware of the @ATTRIBUTE restriction in QGIS now ;-P

@qfield-fairy
Copy link
Collaborator

qfield-fairy commented Sep 1, 2023

Copy link
Collaborator

@suricactus suricactus left a comment

Choose a reason for hiding this comment

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

Code looks good.

Needs a bit of docs here and there.

Also put some braces ( for single line ifs. They are okayish if they are followed by return, break or continue, but become super confusing with other statements and error prone when refactoring.

IMO we should add this as a rule to the linter. Opinions @nirvn @m-kuhn ?

src/qml/LocatorItem.qml Outdated Show resolved Hide resolved
src/qml/LocatorItem.qml Outdated Show resolved Hide resolved
src/core/locator/locatormodelsuperbridge.h Outdated Show resolved Hide resolved
Comment on lines +62 to +65
QStringList prepare( const QString &string, const QgsLocatorContext &locatorContext ) override;
void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
void triggerResultFromAction( const QgsLocatorResult &result, const int actionId ) override;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
QStringList prepare( const QString &string, const QgsLocatorContext &locatorContext ) override;
void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
void triggerResultFromAction( const QgsLocatorResult &result, const int actionId ) override;
QStringList prepare( const QString &string, const QgsLocatorContext &locatorContext ) override;
void fetchResults( const QString &string, const QgsLocatorContext &context, QgsFeedback *feedback ) override;
void triggerResult( const QgsLocatorResult &result ) override;
void triggerResultFromAction( const QgsLocatorResult &result, const int actionId ) override;

A bit of docs would be nice.

Copy link
Member Author

Choose a reason for hiding this comment

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

src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
src/core/locator/activelayerfeatureslocatorfilter.cpp Outdated Show resolved Hide resolved
@nirvn
Copy link
Member Author

nirvn commented Sep 8, 2023

@suricactus , review addressed, thanks a bunch for looking over this.

@nirvn nirvn enabled auto-merge September 8, 2023 01:51
@nirvn nirvn disabled auto-merge September 8, 2023 04:26
@nirvn nirvn merged commit 4aae089 into master Sep 8, 2023
19 checks passed
@nirvn nirvn deleted the activelayer_locator branch September 8, 2023 04:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants