Skip to content

[UUM-133530] Ensure "Set Double Sided" is enabled when active selection is proper#660

Open
thomas-tu wants to merge 2 commits intomasterfrom
bugfix/sample-menuaction-disabled
Open

[UUM-133530] Ensure "Set Double Sided" is enabled when active selection is proper#660
thomas-tu wants to merge 2 commits intomasterfrom
bugfix/sample-menuaction-disabled

Conversation

@thomas-tu
Copy link
Collaborator

@thomas-tu thomas-tu commented Mar 13, 2026

DO NOT FORGET TO INCLUDE A CHANGELOG ENTRY

Purpose of this PR

Implementation of the menu action was missing two elements:

  • call to base.enabled that does some extra checks to validate that the selection context is correct with the provided validSelectModes. It's a common pattern in ProBuilder (look at all PB's menu action).
  • hasFileMenuEntry needs to be set to false. Base implementation returns true but that would indicate that the custom action exists somewhere in the top menu as the code would try to resolve the action by looking up at the one from the top menu.

Links

Jira: UUM-133530

…e enabled in the contextual menu due to a missing override.
Copy link

@u-pr u-pr bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perfect
The PR quality is excellent. Only one minor documentation issue was found.

🤖 Helpful? 👍/👎

@@ -38,9 +38,11 @@ public class MakeFacesDoubleSided : MenuAction
/// <returns></returns>
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

low
The XML documentation tag is empty and is generally used for methods rather than properties. For properties, the

or tags are standard. Consider removing the empty tag to keep the documentation clean.

🤖 Helpful? 👍/👎

Copy link
Contributor

@varinotmUnity varinotmUnity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice, users have been asking for this for a while on forums :)

@codecov-github-com
Copy link

codecov-github-com bot commented Mar 13, 2026

Codecov Report

All modified and coverable lines are covered by tests ✅

@@           Coverage Diff           @@
##           master     #660   +/-   ##
=======================================
  Coverage   36.05%   36.06%           
=======================================
  Files         277      277           
  Lines       34909    34910    +1     
=======================================
+ Hits        12588    12589    +1     
  Misses      22321    22321           
Flag Coverage Δ
probuilder_MacOS_6000.0 34.58% <ø> (ø)
probuilder_MacOS_6000.3 34.58% <ø> (ø)
probuilder_MacOS_6000.4 34.57% <ø> (ø)
probuilder_MacOS_6000.5 34.57% <ø> (ø)
probuilder_MacOS_6000.6 34.57% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants