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

Editor palettes: allow keyboard scrolling to see the last row #6733

Merged
merged 1 commit into from May 31, 2022

Conversation

stevecotton
Copy link
Contributor

Fixes #6724, where scrolling with the mousewheel could reach the last row, but
scrolling with keys or buttons couldn't. Although can_scroll_up() and
can_scroll_down() already existed, can_scroll_down() didn't allow access to the
last line when that line had less than the full number of items. The mousewheel
still worked, because it ignored that logic.

Remove a feature that was seen when the last row is visible - things jumped
between columns to fill the spaces. If you have 10 items, with 4 columns and 2
visible rows then it will appear like this:

    a b c d
    e f g h

when scrolling down it used to move items sideways to fill the last line, which
seems bad UI because it made items harder to find:

    c d e f
    g h i j

the new code is simpler, and will instead show the following when scrolling down:

    e f g h
    i j

Move the layout code into adjust_size(), so that it runs one time when the number
of buttons changes. That could be separated from this commit, but the code
would still be touched in this commit (counter_from_zero would still be
replaced by i), so doing it and testing the changes together made sense.

Note for the master branch only: scrolling the palettes up and down is
noticeably laggy, which is a regression from last week. That's not caused by
this commit, we're just at a point in the rendering refactor where
surface_restorer::update() and thus gui::widget::hide() are slow. The fix
for that is already in progress and going to be in 1.17.5.

Fixes wesnoth#6724, where scrolling with the mousewheel could reach the last row, but
scrolling with keys or buttons couldn't. Although can_scroll_up() and
can_scroll_down() already existed, can_scroll_down() didn't allow access to the
last line when that line had less than the full number of items. The mousewheel
still worked, because it ignored that logic.

Remove a feature that was seen when the last row is visible - things jumped
between columns to fill the spaces. If you have 10 items, with 4 columns and 2
visible rows then it will appear like this:
    a b c d
    e f g h
when scrolling down it used to move items sideways to fill the last line, which
seems bad UI because it made items harder to find:
    c d e f
    g h i j
the new code is simpler, and will instead show the following when scrolling down:
    e f g h
    i j

Move the layout code into adjust_size(), so that it runs one time when the number
of buttons changes. That could be separated from this commit, but the code
would still be touched in this commit (`counter_from_zero` would still be
replaced by `i`), so doing it and testing the changes together made sense.

Note for the master branch only: scrolling the palettes up and down is
noticeably laggy, which is a regression from last week. That's not caused by
this commit, we're just at a point in the rendering refactor where
`surface_restorer::update()` and thus `gui::widget::hide()` are slow. The fix
for that is already in progress and going to be in 1.17.5.
@stevecotton stevecotton added Bug Issues involving unexpected behavior. Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. UI User interface issues, including both back-end and front-end issues. Editor Map/scenario editor issues. labels May 30, 2022
@stevecotton stevecotton self-assigned this May 30, 2022
Copy link
Member

@Wedge009 Wedge009 left a comment

Choose a reason for hiding this comment

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

I confirm the slow-down in the editor scrolling was already present in master. I also confirm this change accomplishes what I would expect the behaviour of the tiles to be when scrolling - thanks for that.

@stevecotton stevecotton merged commit 8edd8f2 into wesnoth:master May 31, 2022
@stevecotton stevecotton deleted the editor_palette_scrolling branch May 31, 2022 12:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Backport A reminder of a bugfix that was added to master that needs to be duplicated on the stable branch. Bug Issues involving unexpected behavior. Editor Map/scenario editor issues. UI User interface issues, including both back-end and front-end issues.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: palette's scroll down button can't scroll to last row of items
3 participants