UI: Add the rest of bevel's options to the active tool
ClosedPublic
Actions

Authored by Hans Goudey (HooglyBoogly) on Sep 17 2019, 10:58 PM.

Details

Reviewers

William Reynish (billreynish)
Pablo Vazquez (pablovazquez)
Campbell Barton (campbellbarton)

Commits

rBd7a8a606889f: UI: Add extra bevel options to popover from tool settings bar

Summary

The bevel active tool currently only has a small set of the complete bevel options. The active tool would be much more useful if it had all of the options.

The large number of options necessitates creating a popover to that the tool settings header doesn't get too long.

Diff Detail

Repository: rB Blender

Event Timeline

Hans Goudey (HooglyBoogly) created this revision.Sep 17 2019, 10:58 PM

Hans Goudey (HooglyBoogly) edited the summary of this revision. (Show Details)Sep 17 2019, 11:39 PM

what about bevel weights? should it be there in edit mode?

If I understand you correctly, that's separate from the Bevel active tool. The bevel weight is in the "Item" tab of the viewport side panel.

yes that's correct. Bevel weights is about as far away from the bevel tool or modifier as possible. It requires looking for in the ui each time you need to use it. It always made me think, why is such a heavily relied upon bevel tool out there all lonely?

Well the bevel active tool doesn't use the bevel weights, so I'm not sure why it would make sense to have them in the same location. Plus they're very different things-- one is the options for the operation you're about to do with the tool, and one is used to control data attached to edges.

hi, @Hans Goudey (HooglyBoogly), fair enough. I'll just add a pic here that shows there's still a relationship between the edges data and the bevel tool. Fortunately you can access both at once and make changes to/in both ui positions without losing the bevel operator menu. I was thinking more of the modifier tbh and grouping of related items for access in the one place.

Yeah, there's always the battle between convenience (having properties everywhere they might be useful) and organization (keeping only a type of property in a single place.

Added custom profile widget and option to settings

Removed "Face Strength Mode." As an advanced option, it's probably better that this is accessed through the tool or modifier.

@William Reynish (billreynish) Do you have time to look at this before BCon2? It's a pretty small change, and sort of silly these options aren't there already.

Seems fine to add this, but it does reveal some UI issues. Popovers and modifiers cannot use sub-panels currently, which makes this more messy that it needs to be. But, we could add this anyway, and just accept that the UI is not the cleanest, and then updated it when we make the modifiers node-based, and when modifiers are re-worked.

Think we should make the popover name unique. This makes reading/writing docs more difficult if we have multiple "Options" popovers in the same header.

Campbell Barton (campbellbarton) requested changes to this revision.Dec 12 2019, 6:02 AM

Campbell Barton (campbellbarton) added inline comments.

release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
455	Would rather not define a panel type for each instance where options need to overflow into a panel. We could instead: Define a single generic panel type. This calls the tools `draw_settings` with `extra=True`. The draw_settings then handled drawing both the popover content and the layout in the toolbar.
467–484	This looks like it could be de-duplicated with the UI layout below.

This revision now requires changes to proceed.Dec 12 2019, 6:02 AM

The popover is generic now, and could be applied to any tool that has too many settings for the header.
The drawing code is de-duplicated.

Hans Goudey (HooglyBoogly) marked 2 inline comments as done.Dec 12 2019, 8:22 AM

IMO, we should call this 'More' instead of 'Options', since it's in a bar full of options already. But in general I think it's better to use specific descriptive titles, such as Profile, Miter, etc. Not sure if that can fit here in a good way.

Generally looks good, comments inline.

release/scripts/startup/bl_ui/space_toolsystem_toolbar.py
452	Since this is used for the tool header and doesn't belong to a space-type, prefer: `TOPBAR` prefix instead of `TOOLSYSTEM`, then this can be assigned to the `TOPBAR` space type too, instead of the 3D view, since this may be used elsewhere. Realize this isn't exactly correct since it's the tool header, not the topbar space - it's just less confusing than assigning it the 3D view, then using in the image/node space for e.g.
473	Don't think the None check is needed as this should always be created from the `draw_settings` callback.
616	Ensure keyword only argument by using `tool, *, extra=False`
637	Text for this could be an ellipsis (`...`), since there isn't much space in the UI here, I've seen this used before as a way to access collapsed areas of the UI.

This revision now requires changes to proceed.Dec 12 2019, 8:32 AM

Addressed comments.

Thanks for the review!

Hans Goudey (HooglyBoogly) marked an inline comment as done.Dec 12 2019, 8:44 AM

Campbell Barton (campbellbarton) accepted this revision.Dec 12 2019, 11:16 AM

This revision is now accepted and ready to land.Dec 12 2019, 11:16 AM

Looks good, final note. Prefer to keep space_toolsystem_toolbar.py only for tools and not mix in other utility classes.

TOPBAR_PT_extra_options can go in space_topbar.py as TOPBAR_PT_tool_settings_extra we have similar kinds of classes there TOPBAR_PT_tool_fallback, TOPBAR_PT_name, TOPBAR_PT_annotation_layers.