UI: Add Placement Settings to the Options popover #107854

Germano Cavalcante · 2023-05-11T22:38:42+02:00

2023-05-11 22:38:42 +02:00

Since a generic snap cursor was implemented (which can be used by Tools and by DragDrop), the Placement Settings are no longer properties of the "VIEW3D_OT_interactive_add" operator.

They now are part of the ToolSettings and can be used for:

Drag and Drop
Measure Tool
Placement Tool

This makes the interface confusing, as the user doesn't know where to edit the Placement Settings for Drag and Drop.

The solution proposed in this PR is to add the Placement Settings to the Options popover (top right hand corner).

Before:	After:

Since a generic snap cursor was implemented (which can be used by Tools and by DragDrop), the Placement Settings are no longer properties of the "VIEW3D_OT_interactive_add" operator. They now are part of the `ToolSettings` and can be used for: - Drag and Drop - Measure Tool - Placement Tool This makes the interface confusing, as the user doesn't know where to edit the Placement Settings for Drag and Drop. The solution proposed in this PR is to add the Placement Settings to the Options popover (top right hand corner). --- <table> <thead> <tr> <th>Before:</th> <th>After:</th> </tr> </thead> <tbody><tr> <td valign="top"><img src="/attachments/5979af91-1063-4ea5-8ecb-c4c41e3fc6d2" alt="Options Before"></td> <td valign="top"><img src="/attachments/2321e0cf-fc17-4254-871b-eec8ef61f02f" alt="Options After"></td> </tr> <tr> <td valign="top"><img src="/attachments/e05e19f0-3ea8-4622-8a65-38ae47dcd730" alt="Tool Before"></td> <td valign="top"><img src="/attachments/a6e301f2-b41c-4587-a849-bbd29f9d8b60" alt="Tool After"></td> </tr> </tbody></table>

image.png

19 KiB

image.png

56 KiB

image.png

15 KiB

image.png

80 KiB

🚀 1

Germano Cavalcante added the

Module

User Interface

label 2023-05-11 22:38:42 +02:00

Germano Cavalcante requested review from Campbell Barton 2023-05-11 22:39:03 +02:00

Campbell Barton commented

2023-05-12 04:26:40 +02:00

In general moving to a tool setting seems fine:

Including these options in the snap menu doesn't seem right (besides it making an already full popover ever bigger), these options don't depend on snapping being enabled. So this could be moved to the "Options" popover (top right hand corner) - at least I think it's worth considering.
The "..." popover is intended for when settings overflow which is no longer the case, we could expand these options although this could be done in a separate commit.

In general moving to a tool setting seems fine: - Including these options in the snap menu doesn't seem right (besides it making an already full popover ever bigger), these options don't depend on snapping being enabled. So this could be moved to the "Options" popover (top right hand corner) - at least I think it's worth considering. - The "..." popover is intended for when settings overflow which is no longer the case, we could expand these options although this could be done in a separate commit.

Germano Cavalcante force-pushed snap_menu_placement from 2b07fbfdeb to f3fb28a46c

2023-05-12 05:46:53 +02:00

Compare

Germano Cavalcante changed title from ~~UI: move Placement Settings to snap menu~~ to UI: move Placement Settings to the Options popover

2023-05-12 05:53:30 +02:00

Germano Cavalcante commented

2023-05-12 05:55:15 +02:00

Placement Settings have been moved to the "Options" popover.
The '"..." popover kept the same for now

Placement Settings have been moved to the "Options" popover. The '"..." popover kept the same for now

Campbell Barton commented

2023-05-12 05:56:48 +02:00

LGTM, I'd like to get sign off from one of the other UI team though (after that the lonely ... menu should be handled too, if not beforehand).

LGTM, I'd like to get sign off from one of the other UI team though (after that the lonely `...` menu should be handled too, if not beforehand).

Campbell Barton requested review from Pablo Vazquez 2023-05-12 05:57:15 +02:00

Campbell Barton requested review from Julian Eisel 2023-05-12 05:57:15 +02:00

Nika Kutsniashvili commented

2023-05-12 11:49:24 +02:00

This is just hiding useful settings.

👍 1

Germano Cavalcante force-pushed snap_menu_placement from f3fb28a46c to a0a0fb5e61

2023-05-12 22:53:16 +02:00

Compare

Julian Eisel commented

2023-05-15 19:48:51 +02:00

@pablovazquez could you look into this?

To me the empty bar with the ... button is concerning. Why should things be put into some "more"-like popover when there's plenty of space available? Seems like an odd design choice.

@pablovazquez could you look into this? To me the empty bar with the `...` button is concerning. Why should things be put into some "more"-like popover when there's plenty of space available? Seems like an odd design choice.

Germano Cavalcante commented

2023-05-15 21:04:31 +02:00

To prioritize the #106558 fix and avoid rushing design discussions (which might not please everyone), I made a new PR (!107951) that brings part of the changes suggested here (moving the Operator properties to the ToolSettings struct) but doesn't change UI in a way that is perceptible to the user.

It's a little weird to show the tool_settings properties in a tool header, but the existing API allows it and it doesn't affect what users are used to.

I also renamed the ... to Origin & Aspect

To prioritize the #106558 fix and avoid rushing design discussions (which might not please everyone), I made a new PR (!107951) that brings part of the changes suggested here (moving the Operator properties to the `ToolSettings` struct) but doesn't change UI in a way that is perceptible to the user. It's a little weird to show the `tool_settings` properties in a tool header, but the existing API allows it and it doesn't affect what users are used to. I also renamed the `...` to `Origin & Aspect`

Nika Kutsniashvili commented

2023-05-15 21:15:55 +02:00

To prioritize the #106558 fix and avoid rushing design discussions (which might not please everyone), I made a new PR (!107951) that brings part of the changes suggested here (moving the Operator properties to the ToolSettings struct) but doesn't change UI in a way that is perceptible to the user.

It's a little weird to show the tool_settings properties in a tool header, but the existing API allows it and it doesn't affect what users are used to.

This I like. I always wondered why Tool settings were exposed like that (ugly too), when Brushes are better defined in Tool properties. If you can make them like brushes - keep useful settings in Horizontal UI and also move them in Tool settings it's a great solution

> To prioritize the #106558 fix and avoid rushing design discussions (which might not please everyone), I made a new PR (!107951) that brings part of the changes suggested here (moving the Operator properties to the `ToolSettings` struct) but doesn't change UI in a way that is perceptible to the user. > > It's a little weird to show the `tool_settings` properties in a tool header, but the existing API allows it and it doesn't affect what users are used to. This I like. I always wondered why Tool settings were exposed like that (ugly too), when Brushes are better defined in Tool properties. If you can make them like brushes - keep useful settings in Horizontal UI and also move them in Tool settings it's a great solution

Germano Cavalcante force-pushed snap_menu_placement from a0a0fb5e61 to ac7d94302e

2023-05-16 15:20:16 +02:00

Compare

Germano Cavalcante changed title from ~~UI: move Placement Settings to the Options popover~~ to WIP: UI: move Placement Settings to the Options popover

2023-05-16 15:20:34 +02:00

Campbell Barton approved these changes 2023-05-18 07:16:51 +02:00

Campbell Barton reviewed 2023-05-18 07:18:19 +02:00

scripts/startup/bl_ui/space_view3d_toolbar.py Outdated

						
				@ -113,0 +107,4 @@

				def draw_placement_options(layout, tool_settings):

				    col = layout.column(align=True)

				    col.separator()

				    col.label(text="Placement")

Campbell Barton commented

2023-05-18 07:18:19 +02:00

The Transform is center aligned, where as the Placement is left aligned which looks a bit off, perhaps both could be aligned left, don't think it's a blocker for this patch though.

The `Transform` is center aligned, where as the `Placement` is left aligned which looks a bit off, perhaps both could be aligned left, don't think it's a blocker for this patch though.

Germano Cavalcante commented

2023-05-18 15:38:49 +02:00

Edit:
Update:
The UI was changed in 560e9c654b so the previous concerns no longer apply.

**Edit:** **Update:** The UI was changed in 560e9c654b so the previous concerns no longer apply.

Germano Cavalcante force-pushed snap_menu_placement from ac7d94302e to dae0ab3fd9

2023-05-19 17:52:11 +02:00

Compare

Germano Cavalcante commented

2023-05-19 18:08:56 +02:00

The UI was edited in 560e9c654b so this PR only proposes to move the settings currently available in the Tools options from Add primitives to the Options popover.

Note in the images that these options can be accessed in 3 different places. That's why they were removed from the Tools header for adding primitives. Avoid repetition exaggeration.

This I like. I always wondered why Tool settings were exposed like that (ugly too), when Brushes are better defined in Tool properties. If you can make them like brushes - keep useful settings in Horizontal UI and also move them in Tool settings it's a great solution

Brushes are often used for sculpting or painting, so it makes sense for them to be displayed in the header.

The Placement settings are less used, so it would be nice to be relatively hidden to avoid polluting the UI.

The UI was edited in 560e9c654b so this PR only proposes to move the settings currently available in the Tools options from Add primitives to the Options popover. Note in the images that these options can be accessed in 3 different places. That's why they were removed from the Tools header for adding primitives. Avoid repetition exaggeration. > This I like. I always wondered why Tool settings were exposed like that (ugly too), when Brushes are better defined in Tool properties. If you can make them like brushes - keep useful settings in Horizontal UI and also move them in Tool settings it's a great solution Brushes are often used for sculpting or painting, so it makes sense for them to be displayed in the header. The Placement settings are less used, so it would be nice to be relatively hidden to avoid polluting the UI.

Germano Cavalcante changed title from ~~WIP: UI: move Placement Settings to the Options popover~~ to UI: move Placement Settings to the Options popover

2023-05-19 23:54:36 +02:00

Germano Cavalcante requested review from Campbell Barton 2023-05-19 23:54:41 +02:00

Germano Cavalcante referenced this pull request

2023-06-07 17:44:17 +02:00

Snap Polishing #108669

Germano Cavalcante referenced this pull request

2023-06-28 21:44:44 +02:00

Measure tool snapping does not work #109454

Brecht Van Lommel commented

2023-08-16 16:16:42 +02:00

Discoverability of these settings for the placement and measure tool seems not great like this. The panel is also the first one in the options, while I think it's less important than for example the mirroring settings.

A solution could be to duplicate the settings. Keep them in the tool settings bar for the placement and measure tool as before, and also have a Drag & Drop panel in the options as the last panel. It's not very elegant but maybe still better for users overall.

Discoverability of these settings for the placement and measure tool seems not great like this. The panel is also the first one in the options, while I think it's less important than for example the mirroring settings. A solution could be to duplicate the settings. Keep them in the tool settings bar for the placement and measure tool as before, and also have a Drag & Drop panel in the options as the last panel. It's not very elegant but maybe still better for users overall.

Germano Cavalcante commented

2023-08-16 17:43:18 +02:00

Good suggestion @brecht. Analyzing.

Currently the measure tool has no settings.
It might be a good idea to put these options in the tool as well (or leave them as they are to avoid repetition)

In the case of placement tools, we just need to prevent repeated options from appearing in the Sidebar and properties editor. But it seems feasible.

Good suggestion @brecht. Analyzing. Currently the measure tool has no settings. It might be a good idea to put these options in the tool as well (or leave them as they are to avoid repetition) In the case of placement tools, we just need to prevent repeated options from appearing in the Sidebar and properties editor. But it seems feasible.

Brecht Van Lommel commented

2023-08-16 19:24:48 +02:00

I would maybe just collapse the Drag & Drop panel by default. It doesn't strictly prevent these being shown twice when using the Placement or Measure tool, but I also don't think that's really a problem.

Germano Cavalcante force-pushed snap_menu_placement from dae0ab3fd9 to df70d37045

2023-08-16 21:09:40 +02:00

Compare

Germano Cavalcante changed title from ~~UI: move Placement Settings to the Options popover~~ to UI: Add Placement Settings to the Options popover

2023-08-16 21:14:36 +02:00

Germano Cavalcante commented

2023-08-16 21:18:06 +02:00

I've updated the code so that the settings in the Placement Tools header are visually unchanged.

But the Placement options that appeared in these Tools have been removed in the Sidebar and in the Properties editor, since they always appear together with the Options panel.

I've updated the code so that the settings in the Placement Tools header are visually unchanged. But the Placement options that appeared in these Tools have been removed in the Sidebar and in the Properties editor, since they always appear together with the Options panel.

Brecht Van Lommel commented

2023-08-17 00:35:55 +02:00

About the "Placement" title for the panel. Personally I would not have been able to guess what these settings do in the options panel. That's why I suggested "Drag & Drop" instead.

There's no use of the term "Placement" elsewhere in the UI to make the connection as far as I can tell, and the tooltips of the properties also don't explain it.

Is there drag & drop functionality in edit mode that uses these settings? If it's only for the tools that already shows these settings I would not be needed in the options panel then.

About the "Placement" title for the panel. Personally I would not have been able to guess what these settings do in the options panel. That's why I suggested "Drag & Drop" instead. There's no use of the term "Placement" elsewhere in the UI to make the connection as far as I can tell, and the tooltips of the properties also don't explain it. Is there drag & drop functionality in edit mode that uses these settings? If it's only for the tools that already shows these settings I would not be needed in the options panel then.

Germano Cavalcante commented

2023-08-17 14:27:42 +02:00

I didn't notice the "Drag & Drop" suggestion. But it's actually a more descriptive and recognizable name than "Placement".

I made a lot of changes thinking about Drag & Drop and Measure Tool:

Rename panel 'Placement' to 'Drag & Drop'
Prioritize Snap Toggle over ToolSettings.plane_depth option (This is what caused the confusion seen in #109454)
Display the Snap to: option in Measure Tool settings (Default or Geometry)
Remove "Drag & Drop" panel from Mesh Edit Options ("Drag & Drop" with snap is not used in Edit mode)
Revert hiding Placement tool options
Add 'DEFAULT_CLOSE' option to 'Drag & Drop' panel

I'll do another PR for the Prioritize 'Snap Toggle' over 'ToolSettings.plane_depth' option change.

I didn't notice the "Drag & Drop" suggestion. But it's actually a more descriptive and recognizable name than "Placement". I made a lot of changes thinking about Drag & Drop and Measure Tool: - Rename panel 'Placement' to 'Drag & Drop' - Prioritize `Snap Toggle` over `ToolSettings.plane_depth` option (This is what caused the confusion seen in #109454) - Display the `Snap to:` option in Measure Tool settings (`Default` or `Geometry`) - Remove "Drag & Drop" panel from Mesh Edit Options ("Drag & Drop" with snap is not used in Edit mode) - Revert hiding Placement tool options - Add 'DEFAULT_CLOSE' option to 'Drag & Drop' panel I'll do another PR for the `Prioritize 'Snap Toggle' over 'ToolSettings.plane_depth' option` change.

👍 1

Germano Cavalcante added a new dependency 2023-08-17 14:38:36 +02:00

#111225 Build: add global precompiled header

Germano Cavalcante removed a dependency 2023-08-17 14:38:58 +02:00

#111225 Build: add global precompiled header

Germano Cavalcante added a new dependency 2023-08-17 14:39:16 +02:00

#111226 Fix #109454: Snapping from Tools doesn't work if Depth option is not 'Surface'

Campbell Barton approved these changes 2023-08-23 09:20:17 +02:00

Julian Eisel commented

2023-08-23 17:22:33 +02:00

Why are these buttons aligned to each other? We don't do that usually unless buttons are strongly related (like array values, start & end frames, etc.). So this shouldn't use alignment IMO, like for comparable UIs:

Why are these buttons aligned to each other? We don't do that usually unless buttons are strongly related (like array values, start & end frames, etc.). So this shouldn't use alignment IMO, like for comparable UIs: <img src="/attachments/e49b3559-148c-449f-823c-f9349c11a259" width="300px"/>

Screenshot from 2023-08-23 17-21-12.png

67 KiB

Germano Cavalcante force-pushed snap_menu_placement from 9a0a9d2c36 to 90072ce451

2023-08-23 20:39:16 +02:00

Compare

Germano Cavalcante commented

2023-08-23 20:41:22 +02:00

Why are these buttons aligned to each other?

I tried to avoid changing too much what already existed.
But, in fact, aligning is an improvement that can be applied here.
Updated the code and images.

> Why are these buttons aligned to each other? I tried to avoid changing too much what already existed. But, in fact, aligning is an improvement that can be applied here. Updated the code and images.

Julian Eisel commented

2023-08-24 11:33:14 +02:00

What I meant is: why is this using align=True?

Also, please avoid force pushing updates after review started. This overrides the patch history, making review harder. For example I have no idea what code changes you did with the latest update :) See https://wiki.blender.org/wiki/Tools/Pull_Requests#Update_a_Pull_Request.

What I meant is: why is this using `align=True`? Also, please avoid force pushing updates after review started. This overrides the patch history, making review harder. For example I have no idea what code changes you did with the latest update :) See https://wiki.blender.org/wiki/Tools/Pull_Requests#Update_a_Pull_Request.

Germano Cavalcante force-pushed snap_menu_placement from 90072ce451 to 9a459d6b82

2023-08-24 19:59:37 +02:00

Compare

Germano Cavalcante commented

2023-08-24 20:03:07 +02:00

I have no idea what code changes you did with the latest update

I did --force again, but I restored all the commits that were before the last --force.
So you can see the the latest update.
9a459d6b82

> I have no idea what code changes you did with the latest update I did `--force` again, but I restored all the commits that were before the last `--force`. So you can see the the latest update. https://projects.blender.org/blender/blender/commit/9a459d6b82fbfb1218a5c0e6f9dc5e55958a3ed9

👍 1

Julian Eisel commented

2023-09-05 16:28:15 +02:00

Why are these buttons aligned to each other? We don't do that usually unless buttons are strongly related (like array values, start & end frames, etc.). So this shouldn't use alignment IMO, like for comparable UIs:

This is still an open question. This shouldn't use align=True IMO as it's inconsistent with similar UIs.

> Why are these buttons aligned to each other? We don't do that usually unless buttons are strongly related (like array values, start & end frames, etc.). So this shouldn't use alignment IMO, like for comparable UIs: > <img src="/attachments/e49b3559-148c-449f-823c-f9349c11a259" width="300px"/> This is still an open question. This shouldn't use `align=True` IMO as it's inconsistent with similar UIs.

Germano Cavalcante added 2 commits 2023-09-05 21:03:10 +02:00

d4897fe8d5 Merge remote-tracking branch 'origin/main' into snap_menu_placement

c82be7839f Use `align=False`

Germano Cavalcante commented

2023-09-05 21:05:40 +02:00

Kind of like when I copied code from other panels in the file, I didn't realize that align=True was being used in a specific way.
Changed to align=False.
Updated images.

Kind of like when I copied code from other panels in the file, I didn't realize that `align=True` was being used in a specific way. Changed to `align=False`. Updated images.

Merge conflict checking is in progress. Try again in few moments.

View command line instructions.

Checkout

From your project repository, check out a new branch and test the changes.

git fetch -u snap_menu_placement:mano-wii-snap_menu_placement

git checkout mano-wii-snap_menu_placement

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

UI: Add Placement Settings to the Options popover #107854

Checkout