[ui] Improve Search Bar component #2581

waaake · 2024-10-24T14:43:29Z

Description

Search bar Updates

Search bar can now be toggled with the Search Button.
The GraphEditor had an obstructive search before and now it is clubbed with the bottom panel options.
The Search Bar now supports "Return Key Press" to move on the next filtered item in the Graph Editor.
Search Bar also gets a clear text action when text is entered in the field.
Updated SearchBar in ImageGallery, Node Editor, 3D Inspector to now be toggled.

cbentejac · 2024-11-12T10:08:54Z

meshroom/ui/qml/Viewer3D/Inspector3D.qml

-                    Layout.rightMargin: 10
-                    Layout.leftMargin: 10
+
+                    toggle: true // enable toggling the actual text field by the search button


Suggested change

toggle: true // enable toggling the actual text field by the search button

toggle: true // enable toggling the actual text field by the search button

isVisible: true

I think the search bar should already be expanded in the case of the 3D inspector. Otherwise, you just have the magnifier icon on the top-left that blocks the entire row while the loaded elements are displayed on the entire row below, and it creates a major imbalance visually speaking.

I also think it'd be better to have some left- and right-margins in this case: the search bar takes up 100% of the horizontal space, which is not the case of the loaded 3D elements, and it also creates an imbalance.

I have reverted the search bar UI on the Inspector 3d to be the default as how it used to be.
Keeping in mind that a toggle feature here does not do us any good, even hidden the search bar takes an entire row.
That should make it better.

cbentejac · 2024-11-12T10:31:18Z

meshroom/ui/qml/Controls/SearchBar.qml

+                anchors.right: parent.right
+                anchors.verticalCenter: parent.verticalCenter


I think some minor right-padding should be added: when the "clear" button appears, the search bar generally has the focus (so is "highlighted") and when hovering over the button, it goes over the highlighting, which is not very pleasing.

Yes, this is now fixed with the change here,

Meshroom/meshroom/ui/qml/Controls/SearchBar.qml

Line 83 in 1a0d2cf

anchors.rightMargin: 2 // Leave a tiny bit of space so that its highlight does not overlap with the boundary of the parent

This change ensures we're leaving sufficient space on the right side as to not overlap with the parent's bounds.
Hoping this makes it better 🙂

cbentejac · 2024-11-12T10:45:03Z

meshroom/ui/qml/Viewer3D/Inspector3D.qml

+                    Layout.minimumWidth: searchBar.width
+                    maxWidth: parent.width


Something around here triggers the following binding loop:

meshroom/ui/qml/Viewer3D/Inspector3D.qml:280:9: QML Group: Binding loop detected for property "implicitWidth"

Inspector 3d gets the default search bar, so this warning doesn't appear anymore.

yann-lty

Works well overall (only minor changes required)!
Side note, the handling of bound iteration is acting weird when we don't have a match in the Graph Editor (but outside the scope of you changes).

yann-lty · 2024-11-28T16:51:50Z

meshroom/ui/qml/Controls/SearchBar.qml

            }
+
+            // Handle enter Key press and forward it to the parent
+            Keys.onPressed: (event)=> {


Could be using the more semantic onAccepted slot to have both Return and Enter key considered
(to work with Enter key on numpad).
The root signal for the search bar could be accepted to match that.

The core signal from the class is updated to be called accepted but using onAccepted instead of Keys.onPressed currently is emitting the signal twice so keeping it same at the moment,
Have updated the event to consider both Return and Number Pad Enter key though.

yann-lty · 2024-11-28T16:52:33Z

meshroom/ui/qml/GraphEditor/GraphEditor.qml

-                }
-                function modelData(item, roleName_) {
-                    return item.model.object[roleName_]
+            Item {


Out of curiosity, could you elaborate on why this extra Item is needed?

I started facing the issue with Qt6 rebase where the width of the panel reacts to the model contents in the repeater, as the filteredNodes repeater has root.graph.nodes as the model, a new node getting add to the graph was increasing the width of the panel and same when the node gets removed, the width would get decreased. Prior to Qt6 this was working as expected without need for the additional Item which is hidden.

meshrec.mp4

Have taken the easy road here, just the previous code with the delegate Item { visible: false } avoids this issue as of now, so we don't have to add an extra item. I see other search mechanisms in Meshroom also user a SortFilterDelegate so keeping it same at the moment.

yann-lty · 2024-11-28T16:58:17Z

meshroom/ui/qml/GraphEditor/GraphEditor.qml

                textField.onTextChanged: navigation.currentIndex = -1
+
+                onReturnPressed: {
+                    nextArrow.clicked()


Could be great to have a dedicated function to change the navigation current index, instead of triggering the event of another button here.

This is updated to now invoke navigation.navigateForward() instead

…the search button. SearchBar gets the clear text feature allowing text to be cleared with a single click. Added accepted signal for the Searchbar with Return and Enter key Press

Search bar is now placed in the bottom pane of the Graph Editor making it non-obstructing for navigating nodes across Graph Editor

Fixed the issue where navigation arrows on the Filtering was allowing to set an index even when the search had null as the result. Accepted signal on the Seach invokes the function rather than invoking the sigal for the button

Node Editor search can now be toggled making it take lesser space by default.

waaake · 2024-12-02T03:18:59Z

Works well overall (only minor changes required)! Side note, the handling of bound iteration is acting weird when we don't have a match in the Graph Editor (but outside the scope of you changes).

Thanks for this feedback, I had noticed it earlier, but left it as is.
Now with the changes to have a separate function navigateForward and navigateBackward, have added a condition to ensure we have something filtered to be able to change the index of the search. That resolves this issue 🙂

waaake requested review from cbentejac and fabiencastan October 24, 2024 14:43

waaake self-assigned this Oct 24, 2024

cbentejac added this to the Meshroom 2024.1.0 milestone Oct 28, 2024

cbentejac requested changes Nov 12, 2024

View reviewed changes

cbentejac changed the title ~~Dev/search bar~~ [ui] Improve Search Bar component Nov 12, 2024

cbentejac reviewed Nov 12, 2024

View reviewed changes

cbentejac approved these changes Nov 15, 2024

View reviewed changes

waaake force-pushed the dev/SearchBar branch from 1a0d2cf to 65660ae Compare November 26, 2024 10:14

waaake marked this pull request as draft November 26, 2024 11:28

waaake force-pushed the dev/SearchBar branch from 65660ae to 37b1b6a Compare November 26, 2024 13:13

waaake marked this pull request as ready for review November 26, 2024 13:35

yann-lty requested changes Nov 28, 2024

View reviewed changes

waaake closed this Nov 30, 2024

waaake force-pushed the dev/SearchBar branch from 49b1e89 to 2a27f4c Compare November 30, 2024 13:16

waaake added 5 commits November 30, 2024 19:53

[ui] SearchBar: Search bar can now be toggled to become visible with …

f78ee9b

…the search button. SearchBar gets the clear text feature allowing text to be cleared with a single click. Added accepted signal for the Searchbar with Return and Enter key Press

[ui] GraphEditor: Graph Editor gets the toggle based search

b0ed5e4

Search bar is now placed in the bottom pane of the Graph Editor making it non-obstructing for navigating nodes across Graph Editor

[ui] GraphEditor: Search filtering fix

3a04ce6

Fixed the issue where navigation arrows on the Filtering was allowing to set an index even when the search had null as the result. Accepted signal on the Seach invokes the function rather than invoking the sigal for the button

[ui] NodeEditor: Implementation of Toggle based Search.

9404b73

Node Editor search can now be toggled making it take lesser space by default.

[ui] ImageGallery: Implementation of Toggle based Search.

c2c32c8

waaake reopened this Dec 1, 2024

yann-lty approved these changes Dec 2, 2024

View reviewed changes

yann-lty merged commit 409c7ec into develop Dec 2, 2024
5 checks passed

yann-lty deleted the dev/SearchBar branch December 2, 2024 10:19

	toggle: true // enable toggling the actual text field by the search button
	toggle: true // enable toggling the actual text field by the search button
	isVisible: true

		anchors.right: parent.right
		anchors.verticalCenter: parent.verticalCenter

Uh oh!

[ui] Improve Search Bar component #2581

[ui] Improve Search Bar component #2581

Uh oh!

Conversation

waaake commented Oct 24, 2024 • edited by cbentejac Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Search bar Updates

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yann-lty left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

waaake commented Dec 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

waaake commented Oct 24, 2024 •

edited by cbentejac

Loading

waaake commented Dec 2, 2024 •

edited

Loading