Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve Room Editor's navigation bar's scrolling #2422

Merged

Conversation

ivan-mogilko
Copy link
Contributor

@ivan-mogilko ivan-mogilko commented May 8, 2024

Fix #2415

Wrote a ToolStripDropDownMenuEx class which extends ToolStripDropDownMenu and amends or overrides its behavior.
Unfortunately had to use various hacks, including getting internal base methods using reflection.

WHAT IS DONE:

  • Scroll dropdown menu with mouse wheel.
  • Allow to change arrow buttons height to a bigger value.
  • Set dropdown height is at least as big to accommodate 16 items at once, that is current max number of walkable areas, etc.

* Don't close the open dropdown menu when hovering over other items in the Address Bar.
EDIT: this fix is temporarily removed, because it turned to be not reliable and causing new problems.

@ivan-mogilko ivan-mogilko force-pushed the 362--trywheelnavbar branch 4 times, most recently from ad9ec6c to 0861e79 Compare May 9, 2024 11:13
@ericoporto
Copy link
Member

Could the arrow button height just be slightly bigger, like 24? 20 is in the almost sweet spot to be able to move the mouse without much effort to the place but I think this little pixels would make it just right.

@ivan-mogilko
Copy link
Contributor Author

Rewrote, adding a new ToolStripDropDownMenuEx class to AGS.Controls, that handles most things.

This implements a ToolStripDropDownMenuEx class, that supports mouse wheel scrolling out of the box.
Use this custom class in AddressBarExt.
@ivan-mogilko ivan-mogilko force-pushed the 362--trywheelnavbar branch 2 times, most recently from 82a7a22 to 05bf49f Compare May 10, 2024 18:43
@ivan-mogilko ivan-mogilko changed the title Experiment: improve Room Editor's navigation bar's scrolling Improve Room Editor's navigation bar's scrolling May 10, 2024
@ivan-mogilko ivan-mogilko marked this pull request as ready for review May 10, 2024 19:29
@ivan-mogilko ivan-mogilko added what: editor related to the game editor context: ui/ux labels May 10, 2024
@ericoporto
Copy link
Member

ericoporto commented May 10, 2024

I have no idea why it works but I tried this diff below and the artifacts when scrolling went away

diff --git a/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs b/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
index 4afe1ef25..695d975f8 100644
--- a/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
+++ b/Editor/AGS.Controls/Controls/ToolStripDropDownMenuEx.cs
@@ -101,6 +101,16 @@ namespace AGS.Controls
             MinDisplayedItems = 0;
         }

+        protected override CreateParams CreateParams
+        {
+            get
+            {
+                CreateParams cp = base.CreateParams;
+                cp.ExStyle |= 0x02000000;  // Turn on WS_EX_COMPOSITED
+                return cp;
+            }
+        }
+
         /// <summary>
         /// Raises the System.Windows.Forms.ToolStripDropDown.Closing event.
         /// Here we intercept certain scenarios where we do not want our dropdown to close.

From https://stackoverflow.com/a/3718648, I had tried the code in the question which is what we use for our buffered panel in the room editor, but for some reason it doesn't work in this control specifically. More info in the MS docs here.

@ivan-mogilko
Copy link
Contributor Author

So, unfortunately, my fix to "Don't close the open dropdown menu when hovering over other items" is not reliable, and has 2 unexpected consequences:

  1. You have to click twice on another "..." before it displays its dropdown menu
  2. Sometimes the previous dropdown is not closed even when a new one is opened.

Use this to ensure at least 16 visible items at once: that is currently a number of walkable areas, regions and walk-behinds.
@ericoporto
Copy link
Member

ericoporto commented May 11, 2024

There is a comment here in the docs.

Be sure to handle the special case that occurs if the "One screen at a time" mouse option is selected. In this case, the MouseWheelScrollLines property has a value of -1.

Perhaps if bigger than 16 or lower than 0 it's simply set to 16.

Edit: found examples in new Winforms source here and here. The only difference is an internal wheel scroll accumulated value.

@ivan-mogilko
Copy link
Contributor Author

@Crystal-Shard, I tried to make scrolling speed relative to the system settings, using a formula found elsewhere, but I am not certain if did this right, because I don't know what is considered to be a "line" in there. Is it a line of pixels, or a line of text? And if it's a line of text, then should this equal to a single item?

That's how I calculate it now:

int linesPerWheelDelta = SystemInformation.MouseWheelScrollLines;
int heightOfLine = ItemHeight;
int scrollAmount = (linesPerWheelDelta * heightOfLine * e.Delta / SystemInformation.MouseWheelScrollDelta);

On my PC this results in approximately 3 items long scroll at a single wheel tick.

@ivan-mogilko ivan-mogilko force-pushed the 362--trywheelnavbar branch 2 times, most recently from f219712 to 9ec2f4e Compare May 11, 2024 15:35
@ivan-mogilko
Copy link
Contributor Author

ivan-mogilko commented May 11, 2024

I had to calculate number of displayed items, in order to properly support "One screen at a time" .

16 items is not a hardcoded value, but an optional property in this class, so I cannot rely on that.

@ericoporto
Copy link
Member

ericoporto commented May 11, 2024

This works fine here too. I don't think many people set this (one screen) specific setting on windows other than for accessibility reasons. I put a reference to two examples in Winforms source code and it pretty much matches your code - it's written in a much more convoluted way though, but I think it looks fine as is now and it's made in a proper way that minor tweaks are easier to do on top of your code if needed in the future before a new control arises.

@ivan-mogilko ivan-mogilko merged commit a1c3765 into adventuregamestudio:master May 27, 2024
21 checks passed
@ivan-mogilko ivan-mogilko deleted the 362--trywheelnavbar branch May 27, 2024 04:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
context: ui/ux what: editor related to the game editor
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Room editor's navigation bar: pulldown menu is inconvenient to scroll
2 participants