godot icon indicating copy to clipboard operation
godot copied to clipboard

Fix `ScrollBar` grabber mouse input ignores scroll content margins.

Open WhalesState opened this issue 1 year ago • 1 comments

  • Fixes: https://github.com/godotengine/godot/issues/98004

Fixes this, the grab area now will respect the left and top content margins.

image

Also ignoring the focus style content margins and using only the scroll style's one, to prevent the grabber from changing it's position when focused.

Before:

https://github.com/user-attachments/assets/86948e48-1478-4dec-92c1-c0a165cce8fc

After:

https://github.com/user-attachments/assets/13eb758a-1c92-4741-adc1-cbe4cf87b4a7

WhalesState avatar Oct 09 '24 20:10 WhalesState

Grab area seems to be still a bit off (red area works, the rest is still click-through):

The MRP that he has added includes expand margin for both left and right sides in the grabber, if you check it again, you will see that it's actually centered but the first and last 8 pixels can't be clicked. please test with this MRP after removing the expand margins test-scrollbar-grabber-main.zip and try to add some content margins for the scrolls.

The issue was explained in this comment https://github.com/godotengine/godot/issues/98004#issuecomment-2402737603, The ScrollBar doesn't take the content margins into account for mouse input.

WhalesState avatar Oct 10 '24 13:10 WhalesState

Why was this closed? Either way, this looks entirely salvageable.

Mickeon avatar Oct 25 '24 22:10 Mickeon

@Mickeon Guy closed all his PRs over fears of Github censorship, or that's what I got from his a bit rambly reply on another PR

Zireael07 avatar Oct 26 '24 07:10 Zireael07

I don't see any difference with and without this PR. The original MRP is still broken, the new MRP works properly without the fix. It would be nice to see some project that works incorrectly on master, but is fixed by this PR.

KoBeWi avatar Nov 24 '24 23:11 KoBeWi

Here it is, this will show the issue when you try to drag the scroll-bar, the scroll style just have some margins, test with the current godot version and with this PR MRP.

https://github.com/user-attachments/assets/24995801-4d60-4946-b87c-1ec95556fe03

WhalesState avatar Nov 26 '24 11:11 WhalesState

I confirmed that this fixes a bug, but not the linked issues.

Though there are some changes that I'm not sure about. Removing bg makes the margins of focus StyleBox ignored.

KoBeWi avatar Nov 26 '24 12:11 KoBeWi

I confirmed that this fixes a bug, but not the linked issues.

Though there are some changes that I'm not sure about. Removing bg makes the margins of focus StyleBox ignored.

You can define the margins inside an empty stylebox, just test again in the current godot's dev branch with a focus style that has some content margins and you will see the grabber runs into a new position after you grab it. also we are using the scroll style by default inside input, so if i revert this change i will have to respect the focus style's margin inside the input function.

WhalesState avatar Nov 26 '24 12:11 WhalesState

I don't see any difference with and without this PR. The original MRP is still broken, the new MRP works properly without the fix. It would be nice to see some project that works incorrectly on master, but is fixed by this PR.

This issue was mentioned in the linked issue, also i have helped him to fix the original issue. so we can close the issue after merging this PR.

WhalesState avatar Nov 26 '24 12:11 WhalesState

Thanks!

Repiteo avatar Nov 27 '24 16:11 Repiteo