Details

Reviewers

Maniphest Tasks

T77408: Continued Outliner Improvements Design

Commits

rB9de18c361bb4: Outliner: Use `shift` for restrict button child toggle
rB70151e41dc02: Outliner: Left and right walk navigation

Summary

These are small isolated fixes and I figured it would be okay to place them in one patch and then commit them separately. I'll make multiple patches if needed.

Add a filter to hide the row icons but still show the children when the parent is expanded. This prevents the inline row icons from drawing. When a subtree is collapsed, the objects, materials, modifiers, etc. are drawn inline to the right of the parent's name. So this option simply hides those inline icons in the case that those icons are an unnecessary visual clutter.

Use shift for consistency with objects and collections for recursive visibility & selectability toggling.

Add the option to walk up and down the tree with the left and right arrow keys. Previously the left/right keys would only open and close the active tree element, now a right walk will move the active into the subtree after the parent is expanded, and a left arrow will walk to the parent.

Diff Detail

Repository: rB Blender

Event Timeline

Nathan Craddock (natecraddock) requested review of this revision.Aug 20 2020, 1:29 AM

Nathan Craddock (natecraddock) created this revision.

Harbormaster completed remote builds in B9673: Diff 27952.Aug 20 2020, 1:29 AM

Nathan Craddock (natecraddock) mentioned this in T77408: Continued Outliner Improvements Design.Aug 20 2020, 1:30 AM

Nathan Craddock (natecraddock) added a project: User Interface.

Code wise this looks good to me. Didn't find any bugs while testing.

There's however the question if we should still allow shortcut tweaks like here at this point.
@Pablo Vazquez (pablovazquez), @Brecht Van Lommel (brecht), @Dalai Felinto (dfelinto), opinion?

source/blender/makesrna/intern/rna_space.c
3097 ↗	(On Diff #27952)	It's unclear what "Row Children" are I think. How about something like "Inline Children"?

This revision is now accepted and ready to land.Aug 20 2020, 8:37 PM

Rename: Row children to Inline Children

Agreed, "Inline" is a better term here.

Harbormaster completed remote builds in B9700: Diff 27999.Aug 20 2020, 9:00 PM

Nathan Craddock (natecraddock) marked an inline comment as done.Aug 20 2020, 9:04 PM

Hi Nathan, thanks for the patch. A few considerations:

These fixes should be in 2.91, not 2.90
It is ok to have them reviewed together, but when committing do one commit per fix please
"Add a filter to hide the row icons but still show the children when the parent is expanded." can you elaborate on that? I can't understand what it does just by reading this line (nor why this is needed). Can you share a screenshot?
"Add the option to walk up and down the tree with the left and right" - not clear to me what this does. Is this to jump from child to parent?

Regarding Show Inline Children:

What is the motivation for having this setting?
Does it only apply to scene and view layer display modes, or also others? And if so, is the option visible in those cases?
The description can be improved, rather than using "inline" it could explain what that means.

Fix: Walk right opening elements with empty subtrees

Harbormaster completed remote builds in B9719: Diff 28035.Aug 21 2020, 5:01 PM

In D8650#211239, @Dalai Felinto (dfelinto) wrote:

These fixes should be in 2.91, not 2.90

It is ok to have them reviewed together, but when committing do one commit per fix please

Already planning on it :)

Sorry for the vague description of the changes.

"Add a filter to hide the row icons but still show the children when the parent is expanded." can you elaborate on that? I can't understand what it does just by reading this line (nor why this is needed). Can you share a screenshot?

This was something that William and I discussed briefly a while back. He had the idea to create a new filter to hide the inline row icons that show the children of collapsed tree elements. It was such a small change I thought I would include it for review, but it is not vital, just something that might be nice. Don't have a strong opinion on including this.

The default scene with this enabled shows no row/inline icons, but the subtree is still included (just collapsed).

In D8650#211266, @Brecht Van Lommel (brecht) wrote:

Does it only apply to scene and view layer display modes, or also others? And if so, is the option visible in those cases?

Looking at this again, it shouldn't be a filter, but rather a display option. In that case it should be visible in modes other than view layer display.

"Add the option to walk up and down the tree with the left and right" - not clear to me what this does. Is this to jump from child to parent?

In master the left and right arrows only open/close the active tree element. With this change, a right arrow press when the subtree is open will walk into the subtree, and a left arrow press will walk to the parent element.

Nathan Craddock (natecraddock) edited the summary of this revision. (Show Details)Aug 21 2020, 5:11 PM

If we don't have a specific reason to add the Inline Children option, I would just leave it out. It has no obvious place in the UI since the filter popover only exists for scene and view layer modes.

@Brecht Van Lommel (brecht) thanks for taking a look. I agree; I'll remove the inline filter option.

In that case, I'll commit the walk navigation and recursive toggling fix later today

If there's not good reason to have the inline icons toggle, +1 on removing it from the patch.

I would still like to hear opinions about the shortcut change though (@Pablo Vazquez (pablovazquez), @Brecht Van Lommel (brecht), @Dalai Felinto (dfelinto)). I remember Ton being quite clearly against changes to existing shortcuts so long after the 2.80 release.
To be clear, this patch changes it so ⇧ is used rather than Ctrl for recursive toggling.
This is a muscle memory breaking change. But we could argue that it's just a fix for consistency.

In D8650#212739, @Julian Eisel (Severin) wrote:

I would still like to hear opinions about the shortcut change though (@Pablo Vazquez (pablovazquez), @Brecht Van Lommel (brecht), @Dalai Felinto (dfelinto)). I remember Ton being quite clearly against changes to existing shortcuts so long after the 2.80 release.
To be clear, this patch changes it so ⇧ is used rather than Ctrl for recursive toggling.
This is a muscle memory breaking change. But we could argue that it's just a fix for consistency.

It changes it for bones to be consistent with objects and collections, I would consider that a bugfix that is fine to commit.

Remove inline child option

Harbormaster completed remote builds in B9816: Diff 28196.Aug 26 2020, 6:44 PM

I agree that changing ctrl to shift is a good fix for consistency despite changing the shortcut.

Just for completeness, I first became aware of the bone recursive toggling issue from a user on devtalk. The manual and tooltip mentioned nothing about this being a feature so I think switching the shortcut may not be disruptive at all.

I'll make sure that the tooltip and manual are updated to mention that this recursive toggling is also supported for bones

Ah, I missed that it's only changing it for bones. In that case indeed, this is definitely more of a fix that I think is fine to do at this point.

Closed by commit rB70151e41dc02: Outliner: Left and right walk navigation (authored by Nathan Craddock (natecraddock)). · Explain WhyAug 26 2020, 7:25 PM

This revision was automatically updated to reflect the committed changes.

Nathan Craddock (natecraddock) added a commit: rB70151e41dc02: Outliner: Left and right walk navigation.

Nathan Craddock (natecraddock) added a commit: rB9de18c361bb4: Outliner: Use `shift` for restrict button child toggle.

Julian Eisel (Severin) added a task: T77408: Continued Outliner Improvements Design.Sep 10 2020, 2:48 PM