UI: Add Placement Settings to the Options popover #107854

Open
Germano Cavalcante wants to merge 9 commits from mano-wii/blender:snap_menu_placement into main

When changing the target branch, be careful to rebase the branch in your fork to match. See documentation.

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:
Options Before Options After
Tool Before Tool 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>
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

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
Author
Member

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

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
Contributor

This is just hiding useful settings.

This is just hiding useful settings.
Germano Cavalcante force-pushed snap_menu_placement from f3fb28a46c to a0a0fb5e61 2023-05-12 22:53:16 +02:00 Compare
Member

@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.
Author
Member

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`
Contributor

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
@ -113,0 +107,4 @@
def draw_placement_options(layout, tool_settings):
col = layout.column(align=True)
col.separator()
col.label(text="Placement")

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.
Author
Member

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
Author
Member

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

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.
Author
Member

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.

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.

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
Author
Member

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.

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.
Author
Member

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.
Germano Cavalcante added a new dependency 2023-08-17 14:38:36 +02:00
Germano Cavalcante removed a dependency 2023-08-17 14:38:58 +02:00
Campbell Barton approved these changes 2023-08-23 09:20:17 +02:00
Member

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"/>
Germano Cavalcante force-pushed snap_menu_placement from 9a0a9d2c36 to 90072ce451 2023-08-23 20:39:16 +02:00 Compare
Author
Member

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.
Member

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
Author
Member

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
Member

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
Author
Member

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.

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 Label
Interest
Alembic
Interest
Animation & Rigging
Interest
Asset Browser
Interest
Asset Browser Project Overview
Interest
Audio
Interest
Automated Testing
Interest
Blender Asset Bundle
Interest
BlendFile
Interest
Collada
Interest
Compatibility
Interest
Compositing
Interest
Core
Interest
Cycles
Interest
Dependency Graph
Interest
Development Management
Interest
EEVEE
Interest
EEVEE & Viewport
Interest
Freestyle
Interest
Geometry Nodes
Interest
Grease Pencil
Interest
ID Management
Interest
Images & Movies
Interest
Import Export
Interest
Line Art
Interest
Masking
Interest
Metal
Interest
Modeling
Interest
Modifiers
Interest
Motion Tracking
Interest
Nodes & Physics
Interest
OpenGL
Interest
Overlay
Interest
Overrides
Interest
Performance
Interest
Physics
Interest
Pipeline, Assets & IO
Interest
Platforms, Builds & Tests
Interest
Python API
Interest
Render & Cycles
Interest
Render Pipeline
Interest
Sculpt, Paint & Texture
Interest
Text Editor
Interest
Translations
Interest
Triaging
Interest
Undo
Interest
USD
Interest
User Interface
Interest
UV Editing
Interest
VFX & Video
Interest
Video Sequencer
Interest
Virtual Reality
Interest
Vulkan
Interest
Wayland
Interest
Workbench
Legacy
Blender 2.8 Project
Legacy
Milestone 1: Basic, Local Asset Browser
Legacy
OpenGL Error
Meta
Good First Issue
Meta
Papercut
Meta
Retrospective
Meta
Security
Module
Animation & Rigging
Module
Core
Module
Development Management
Module
EEVEE & Viewport
Module
Grease Pencil
Module
Modeling
Module
Nodes & Physics
Module
Pipeline, Assets & IO
Module
Platforms, Builds & Tests
Module
Python API
Module
Render & Cycles
Module
Sculpt, Paint & Texture
Module
Triaging
Module
User Interface
Module
VFX & Video
Platform
FreeBSD
Platform
Linux
Platform
macOS
Platform
Windows
Priority
High
Priority
Low
Priority
Normal
Priority
Unbreak Now!
Status
Archived
Status
Confirmed
Status
Duplicate
Status
Needs Info from Developers
Status
Needs Information from User
Status
Needs Triage
Status
Resolved
Type
Bug
Type
Design
Type
Known Issue
Type
Patch
Type
Report
Type
To Do
No Milestone
No project
No Assignees
5 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Reference: blender/blender#107854
No description provided.