Page MenuHome

Make down the default expand direction for RNA Enum
ClosedPublic

Authored by Mikhail Rachinskiy (alm) on Feb 15 2019, 2:13 PM.

Details

Summary

Make down the default expand direction for RNA Enum dropdown menu.

RNA Enum is the only list type widget that expands up by default, that deceives user expectations and makes it hard to work with when two different types of dropdowns located next to each other.

What this patch does:

  • Removes special treatment of expand direction for RNA Enum.
  • Adds layout flip for RNA Enum.

RNA Enum menu assembled in reverse, which is why I have to flip its layout because the default direction is now opposite.

Patch preview:

Diff Detail

Repository
rB Blender

Event Timeline

I agree with this. I always found it very curious that the enums open upwards by default.
Others?

This revision is now accepted and ready to land.Feb 15 2019, 2:16 PM

Sounds like an improvement for sure! I remember it being mentioned in the past multiple times.

I can't test the patch right now, does it also affect color picker-type popups? And I guess the behavior that if there's no room available it will open upwards is still there.

Thanks for looking into this!

does it also affect color picker-type popups?

It does not, color picker is a different widget type.

if there's no room available it will open upwards is still there.

Of course! The patch affects only the default expand direction in case where there is enough space in both directions.

does it also affect color picker-type popups?

It does not, color picker is a different widget type.

Good! Even though it'd be the only widget type opening upwards, it's better it doesn't. At least given the current color-picker design, once it gets re-designed we can consider opening it downwards to match the rest.

if there's no room available it will open upwards is still there.

Of course! The patch affects only the default expand direction in case where there is enough space in both directions.

Then LGTM! Up to the developers now to review the code.

Just to add some more flavour here.

I think ideally we would handle enum menus even more differently. Here's how:

  • Don't open up or down, but open out from the selected item.
  • Add a checkbox to the item that is selected, so you know what was chosen before

This is more compact and more visually logical, avoiding the doubling of the text for the chosen item.

Example:

But that would also require to write a new widget type, which is far beyond the scope of this patch.

Yes, that would be a lot more work to do. Still, it’s something to keep in mind for later. For now I think opening downwards is at least nicer.

Not sure if it can be addressed by this patch, but it seems related.
In the RGB curves node, the wrench icon menu opens sideways to the right of the button, unlike any other, which can get annoying.

Not sure if it can be addressed by this patch, but it seems related.
In the RGB curves node, the wrench icon menu opens sideways to the right of the button, unlike any other, which can get annoying.

It's a different widget type, so it's not related to this patch.

PS: Please always use 2.8 for screenshots as it can get confusing for users and developers.

PS: Please always use 2.8 for screenshots as it can get confusing for users and developers.

Apologies, I see now it is no longer an issue; sorry for the noise, didn't have a 2.8 build at hand currently to demo

Putting the enum menu title at the bottom is a bit odd, but seems fine to me anyway.

source/blender/editors/interface/interface_region_menu_popup.c
225

Was there a reason to change the test that distinguishes between a pulldown and other menus? The pup->but->type == UI_BTYPE_PULLDOWN part was left out here.

Putting the enum menu title at the bottom is a bit odd, but seems fine to me anyway.

This is how it always been implemented, my patch does not influence menu title positioning.

source/blender/editors/interface/interface_region_menu_popup.c
225

I did not see the reason to keep it, both tests do well on their own. I can replace current test with pup->but->type != UI_BTYPE_PULLDOWN and the result would not change.

Brecht Van Lommel (brecht) requested changes to this revision.Feb 15 2019, 6:26 PM

This is how it always been implemented, my patch does not influence menu title positioning.

Right, it's just a consequence of the patch that the title is now at the bottom most of the time instead of the top. Which I think is acceptable, but it's a slight downside.

source/blender/editors/interface/interface_region_menu_popup.c
225

For the majority of menus it's the same, but for some corner cases like the menu in uiTemplateTextureUser it's not.

Please use the same test as before.

This revision now requires changes to proceed.Feb 15 2019, 6:26 PM

Add test for pulldown menu type.

Mikhail Rachinskiy (alm) marked 2 inline comments as done.Feb 15 2019, 6:35 PM
This revision is now accepted and ready to land.Feb 15 2019, 6:46 PM

Since I do not have commit rights, would anyone want to commit this one?

This revision was automatically updated to reflect the committed changes.