Brush Assets: Highlight first brush on type to search, activate with enter #123853

Julian Eisel · 2024-06-27T19:05:48+02:00

Julian Eisel commented

2024-06-27 19:05:48 +02:00

This is implemented as a general view feature now. Whenever filtering is used, we attempt to highlight the first item so enter can activate it. Note that this still only works in the asset shelf popup so far though, because of the whole "semi-modal" text button that allows events to be passed to other buttons.

Moving the mouse makes the highlight jump back to the brush under the cursor again. This is how search menus behave too.

Technical changes:

Refactors view item filtering, so it's built in deeper into the general view code and happens via an explicit function call. This way the highlighting logic can be handled in a well defined manner.
Assign brush activation operator to the view-item instead of the preview tile button. Otherwise handling the enter key through the view item wouldn't call it.
Makes preview-tile buttons non-interactive so all events are passed through to the overlapped view item button. This way "Assign Shortcut" and the like keep working with the operator assigned to the view item instead of the preview tile. I think this makes sense conceptually.
Ensure the optimization to only create view item buttons for items scrolled into view forces the highlighted button to be visible. The view scrolling should be reset on text input.

This is implemented as a general view feature now. Whenever filtering is used, we attempt to highlight the first item so enter can activate it. Note that this still only works in the asset shelf popup so far though, because of the whole "semi-modal" text button that allows events to be passed to other buttons. Moving the mouse makes the highlight jump back to the brush under the cursor again. This is how search menus behave too. Technical changes: - Refactors view item filtering, so it's built in deeper into the general view code and happens via an explicit function call. This way the highlighting logic can be handled in a well defined manner. - Assign brush activation operator to the view-item instead of the preview tile button. Otherwise handling the enter key through the view item wouldn't call it. - Makes preview-tile buttons non-interactive so all events are passed through to the overlapped view item button. This way "Assign Shortcut" and the like keep working with the operator assigned to the view item instead of the preview tile. I think this makes sense conceptually. - Ensure the optimization to only create view item buttons for items scrolled into view forces the highlighted button to be visible. The view scrolling should be reset on text input.

Julian Eisel added 6 commits 2024-06-27 19:05:58 +02:00

Refactor view filtering for more control on filter process 58dff29a2e

Merge branch 'brush-assets-project' into temp-asset-shelf-search-highligh-on-type 84e8ab161a

Merge branch 'brush-assets-project' into temp-asset-shelf-search-highligh-on-type badb570f27

Highlight first item when typing and make sure its in view be7fcfe223

Get activation on enter to work (a bit hacky still) b3630a9b4a

Clear search highlight on mouse move cf5628681f

Julian Eisel added 1 commit 2024-06-27 19:09:32 +02:00

Remove unnecessary const change 0d5e2685dc

Julian Eisel added 1 commit 2024-06-28 12:31:29 +02:00

Merge branch 'brush-assets-project' into temp-asset-shelf-search-highligh-on-type 55e4922da7

Julian Eisel added 1 commit 2024-06-28 17:25:19 +02:00

Make preview buttons non-interactive so view item buttons get all events e430f08b50

Fixes operators like "Assign Shortcut" not showing up correctly in the
context menu.

Julian Eisel added 1 commit 2024-06-28 17:28:16 +02:00

Remove hack to set the activate operator for the view item button 99238fb82f

The operator is now always set on the view item button (the preview tile
button passes all events to it) through a proper API call.

Julian Eisel changed title from ~~WIP: Highlight first brush on type to search, activate with enter~~ to Brush Assets: Highlight first brush on type to search, activate with enter

2024-06-28 18:26:24 +02:00

Julian Eisel requested review from Hans Goudey 2024-06-28 18:26:33 +02:00

Julian Eisel added this to the Brush Assets & Asset Shelf project 2024-06-28 18:27:21 +02:00

Julian Eisel referenced this pull request

2024-06-28 18:27:43 +02:00

Brush Assets: Tasks #116337

Julian Eisel added 1 commit 2024-07-01 14:29:05 +02:00

Merge branch 'brush-assets-project' into temp-asset-shelf-search-highligh-on-type 0a7c78860d

Hans Goudey reviewed 2024-07-01 15:30:24 +02:00

Hans Goudey left a comment

It all seems reasonable, just noticed a few smaller style things.

However it seems like it would make more sense to commit this to main than the branch?

It all seems reasonable, just noticed a few smaller style things. However it seems like it would make more sense to commit this to main than the branch?

source/blender/editors/interface/interface_query.cc Outdated

						
				@ -514,0 +515,4 @@

				{

				  return ui_but_find(

				      region,

				      [](const uiBut *but, const void *) {

Hans Goudey commented

2024-07-01 15:25:42 +02:00

Use /*named_argument*/

Use `/*named_argument*/`

JulianEisel marked this conversation as resolved

source/blender/editors/interface/interface_query.cc Outdated

						
				@ -514,0 +520,4 @@

				          return false;

				        }

				        const uiButViewItem *view_item_but = (const uiButViewItem *)but;

Hans Goudey commented

2024-07-01 15:25:30 +02:00

Use reinterpret_cast

Use `reinterpret_cast`

JulianEisel marked this conversation as resolved

source/blender/editors/interface/views/abstract_view.cc Outdated

						
				@ -33,0 +34,4 @@

				{

				  const AbstractViewItem *found_item = nullptr;

				  foreach_view_item([&](const AbstractViewItem &item) {

Hans Goudey commented

2024-07-01 15:25:56 +02:00

this->foreach_view_item

`this->foreach_view_item`

JulianEisel marked this conversation as resolved

source/blender/editors/interface/views/abstract_view.cc Outdated

						
				@ -118,0 +145,4 @@

				  prev_filter_string_ = *filter_str;

				  bool has_search_highlight = false;

				  foreach_view_item([&](AbstractViewItem &item) {

Hans Goudey commented

2024-07-01 15:29:35 +02:00

this->

`this->`

JulianEisel marked this conversation as resolved

source/blender/editors/interface/views/abstract_view.cc Outdated

						
				@ -166,0 +212,4 @@

				void AbstractView::clear_search_highlight()

				{

				  foreach_view_item([](AbstractViewItem &item) { item.is_highlighted_search_ = false; });

Hans Goudey commented

2024-07-01 15:29:21 +02:00

this->

`this->`

JulianEisel marked this conversation as resolved

source/blender/editors/interface/views/grid_view.cc

						
				@ -409,2 +449,3 @@

				                                      const View2D &v2d,

				                                      uiLayout &layout)

				                                      uiLayout &layout,

				                                      std::optional<StringRef> search_string)

Hans Goudey commented

2024-07-01 15:27:50 +02:00

I'd put this search string argument before layout to group "const/input" arguments before "outputs" (the layout).

I'd put this search string argument before `layout` to group "const/input" arguments before "outputs" (the layout).

Julian Eisel commented

2024-07-01 17:42:26 +02:00

This is an optional argument, and I'd prefer to keep it that way since it's only used in a few cases.

Julian Eisel commented

2024-07-01 17:37:39 +02:00

However it seems like it would make more sense to commit this to main than the branch?

There might be some conflicts, can try though. Working in the branch is easiest & most efficient for me right now, I can still split off individual changes later. I try to focus on getting through the remaining tasks currently.

> However it seems like it would make more sense to commit this to main than the branch? There might be some conflicts, can try though. Working in the branch is easiest & most efficient for me right now, I can still split off individual changes later. I try to focus on getting through the remaining tasks currently.

Julian Eisel added 1 commit 2024-07-01 17:43:07 +02:00

Address review comments ea16e2cae1

Hans Goudey approved these changes 2024-07-01 18:13:09 +02:00

Hans Goudey left a comment

There might be some conflicts, can try though. Working in the branch is easiest & most efficient for me right now, I can still split off individual changes later. I try to focus on getting through the remaining tasks currently.

Okay, that's fine, I was just thinking about this not ending up in the final brush assets commit. Thanks.

>There might be some conflicts, can try though. Working in the branch is easiest & most efficient for me right now, I can still split off individual changes later. I try to focus on getting through the remaining tasks currently. Okay, that's fine, I was just thinking about this not ending up in the final brush assets commit. Thanks.

Julian Eisel referenced this issue from a commit

2024-07-01 20:38:26 +02:00

UI: Highlight first view item on type to search & activate on enter

Julian Eisel commented

2024-07-01 20:42:15 +02:00

Committed to main with 4a9e8087a7.

Committed to main with 4a9e8087a7.

Julian Eisel closed this pull request

2024-07-01 20:42:19 +02:00

Pull request closed

Please reopen this pull request to perform a merge.

Sign in to join this conversation.

No reviewers

No Label

Download

What's New

Blender Studio

Manual

Developers Blog

Documentation

Benchmark

Blender Conference

Development Fund

One-time Donations

Brush Assets: Highlight first brush on type to search, activate with enter #123853

Pull request closed