UI: Make channel_find use textbox instead of pop-up
ClosedPublic
Actions

Authored by Sybren A. Stüvel (sybren) on Aug 6 2021, 5:08 AM.

Details

Reviewers

Christoph Lendenfeld (ChrisLend)
bird_d (bird_d)

Commits

rB41a4c62c3155: Animation UI: Make Ctrl+F use textbox instead of pop-up

Summary

This makes it less cumbersome to search for channels by
selecting the textbox above the channel list. It feels nicer
to use and has the niceties that come from using that textbox,
like searching per keystroke, compared to the old pop-up method.

This works similarly to how pressing ctrl+f in the text editor
works by having the channel region pop-up and then
select the channel filter textbox during the draw method, if
a flag is set.

This also renames the operator for ctrl + f animation channels to reflect the new function and changes the keymap.

Videos to show difference
Old

New

Diff Detail

Repository: rB Blender

Event Timeline

bird_d (bird_d) requested review of this revision.Aug 6 2021, 5:08 AM

bird_d (bird_d) created this revision.

bird_d (bird_d) edited the summary of this revision. (Show Details)Aug 6 2021, 5:10 AM

Pieter Schiettecatte (Schiette) awarded a token.Aug 6 2021, 1:37 PM

Christoph Lendenfeld (ChrisLend) awarded a token.Aug 7 2021, 3:34 PM

Ankit Meel (ankitm) added a project: Animation & Rigging.Aug 8 2021, 5:45 AM

bird_d (bird_d) added a reviewer: Sybren A. Stüvel (sybren).Aug 9 2021, 10:59 AM

I have to admit I never used this feature before. But comparing it to the way it currently is, this is definitely an improvement.

Minor nitpick: code style is to end comments with a dot

This revision now requires changes to proceed.Aug 9 2021, 11:01 PM

Christoph Lendenfeld (ChrisLend) added inline comments.Aug 9 2021, 11:04 PM

source/blender/editors/animation/time_scrub_ui.c
256–257 ↗	(On Diff #40391)	I am not sure I understand what "trick" means here. Is this flag set as soon as buttons are created?

bird_d (bird_d) added inline comments.Aug 10 2021, 4:55 AM

source/blender/editors/animation/time_scrub_ui.c
256–257 ↗	(On Diff #40391)	To be honest I left it unchanged from copying the text filter select op, I can make it more verbose. The flag gets set during the operator alongside the channel region visibility being toggled if hidden. It gets checked for during the draw call after the UI for the channel filter textbox is created, if true selects that textbox. /* This flag trick is to make sure UI has been added before trying to select the * channel filter textbox. The flag is set during #ANIM_OT_channels_select_filter. */ Would this be better? In D12146#315059, @Christoph Lendenfeld (ChrisLend) wrote: Minor nitpick: code style is to end comments with a dot This is about this comment and the one below, right? I was trying to follow the surrounding code, seemed to have missed this file's, thanks.

Christoph Lendenfeld (ChrisLend) added inline comments.Sep 2 2021, 11:10 PM

source/blender/editors/animation/time_scrub_ui.c
256–257 ↗	(On Diff #40391)	Well it's not really a trick then. I'd keep it short but precise. This flag is set if the UI has been created.

Thanks for the patch @bird_d (bird_d) , it's certainly an improvement over the old situation.

In the new code, ED_time_scrub_channel_search_draw() now activates a text button when the ADS_FLAG_FILTER_ACTIVATE flag is set. I'm not a fan of such an approach, but when I changed the code to just call UI_textbutton_activate_rna() in the operator's exec code, the UI got stuck. I did some more digging, and buttons_start_filter_exec() also calls UI_textbutton_activate_rna() directly, but without hanging all of Blender. The trick was in overriding the region in the context, so that's what I added to animchannels_select_filter_exec() as well.

The second problem to overcome is the un-hiding the sidebar; without redrawing first, it's not possible to activate the input box, as it doesn't exist yet. Instead of adding a flag & doing this in the draw code (I don't like that the text editor does that either), I rewrote the operator to be modal. This means it can keep running at least until the redraw is done, keeping all related code inside that one operator.

I'll take over the patch to get this change in, so that it can be committed before the Blender 3.0 release branch is created.