BrushPopup and Dialog autoscrollbars fixes#5485
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
Window's bounds origin must be set to (0,0) when using multiple displays. I've changed that when introduced the Dialog's autoscrollbars property, without noticing that I broke how the windows were located inside their native window when using multiple displays
When the BrushPopup was open and there was an unlocked slot and then the user pressed the "Save Brush Here" button, the popup was not refreshed
When the Dialog autoscrollbars property was set to true and multiple displays were used, the dialog didn't show up centered on the screen
fa0bbbe to
4c92828
Compare
aseprite-bot
left a comment
There was a problem hiding this comment.
clang-tidy made some suggestions
| if (type == LUA_TBOOLEAN) | ||
| autoScrollbars = lua_toboolean(L, -1); | ||
| if (type == LUA_TBOOLEAN) { | ||
| bool autoScrollbars = lua_toboolean(L, -1); |
There was a problem hiding this comment.
warning: implicit conversion 'int' -> 'bool' [readability-implicit-bool-conversion]
| bool autoScrollbars = lua_toboolean(L, -1); | |
| bool autoScrollbars = lua_toboolean(L, -1) != 0; |
| if (type == LUA_TBOOLEAN) | ||
| autoScrollbars = lua_toboolean(L, -1); | ||
| if (type == LUA_TBOOLEAN) { | ||
| bool autoScrollbars = lua_toboolean(L, -1); |
There was a problem hiding this comment.
warning: variable 'autoScrollbars' of type 'bool' can be declared 'const' [misc-const-correctness]
| bool autoScrollbars = lua_toboolean(L, -1); | |
| bool const autoScrollbars = lua_toboolean(L, -1); |
Try the following in the current Aseprite version:
Then, after applying just the two first commits try the above again. In step 4 you should see that the BrushPopup gets updated. Also try the following script to see the issue that will be fixed after applying the remaining commit: dlg = Dialog { title="Test dialog" }
for i = 1, 39 do
dlg:entry { label="Entry " .. i}
end
dlg:button { text="Apply"}
:button { text="Cancel"}
dlg:show{wait = false, autoscrollbars=true}Once run you will see that the dialog is positioned at (0,0) instead of being centered. Make sure you are using the multiple displays option. This is fixed after applying the last commit. No, it is not related to any of those issues, it is just a fix that makes the dialog with autoscrollbars behave as before reverting the fit_bounds change. |
Thanks! I was able to reproduce the bugs and test the fixes. Looks great 👍 |
This PR contains 2 fixes in three commits:
autoscrollbars=true