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

Allow scrolling of string_input_popup descriptions #58609

Merged

Conversation

ZeroInternalReflection
Copy link
Contributor

@ZeroInternalReflection ZeroInternalReflection commented Jun 22, 2022

Summary

Interface "Allow scrolling of string_input_popup descriptions"

Purpose of change

When creating a string_input_popup window, there is no error-checking to ensure that the window will fit on the screen. So, for string_input_popups with complicated description information (notably the crafting filter &->/), the window will fail to display on small screens. In certain circumstances, this can go on to cause other display problems.

Fixes #48700
Fixes #52688

Describe the solution

  1. Limit the height of a string_input_popup window to TERMY
  2. Move the display of the description to scrolling_text_view (paged with PPAGE/NPAGE, scrolled with Mousewheel), allowing descriptions of arbitrary length

Describe alternatives you've considered

  1. Force a cap on the size of string_input_popup descriptions
    Could cause problems with translation
  2. Trim down the description of the crafting filter (I believe this is the only place the issue currently crops up)
    Not really a long-term solution

Testing

New screenshots/videos 2022-06-23:

Crafting filter in English at various sizes

CraftingFilter--English--80x24
CraftingFilter--English--80x24-2
CraftingFilter--English--80x36

Crafting filter in various non-English languages (Polish pictured) at various sizes

CraftingFilter--Polski--80x24
CraftingFilter--Polski--80x24-2
CraftingFilter--Polski--80x36

Simple string input popup that fits on one line

Change_nickname--80x24

Simple string input popup that has title and input on different lines

Default_character_name--80x24

Simple string input popup where the title folds to a second line

(Yes, it folds like that before this too)
Change_character_name--80x24

Keybindings menu (string_input_popup with custom window)

Keybindings--80x24

String input popup with displayed history

View_item_filter_history--80x24

Scrolling behaviour with mouse and page up/page down (Russian)
Mousewheel_scrolling--Crafting_filter--Russian-v2.mp4
Resizing behaviour - popup without description (Polish)
Resizing_behaviour--Character_name_change--Polish.mp4
Resizing behaviour - Crafting filter
Resizing_behaviour--Crafting_filter.mp4

Additional context

  1. I'm not thrilled with how folded text doesn't match indentation of the line above (see the Polish testing above), but I think that will require extensive tinkering and testing in foldstring().
  2. In screen reader testing (with orca), the screen reader read the visible portion of the crafting screen in addition to the filter description. I'm not sure if this is due to how I've configured orca, or if it's a different underlying problem. However, the behaviour was equivalent both before and after this PR, so at least I haven't broken it more.

@github-actions github-actions bot added [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions labels Jun 22, 2022
src/string_input_popup.cpp Outdated Show resolved Hide resolved
src/string_input_popup.cpp Outdated Show resolved Hide resolved
src/string_input_popup.cpp Outdated Show resolved Hide resolved
src/string_input_popup.cpp Outdated Show resolved Hide resolved
src/string_input_popup.cpp Outdated Show resolved Hide resolved
src/string_input_popup.cpp Outdated Show resolved Hide resolved
@ZeroInternalReflection
Copy link
Contributor Author

Thank you Qrox for the review. I'm sorry there were so many mistakes to find. I believe the latest commits should address all of your comments.
I've tested screen reader behaviour with orca, and aside from it trying to read the screen behind the popup (which is apparently not new to this PR) it seems to be working now.
I've added mousewheel scrolling to the description. I've also moved the keyboard keybindings to PPAGE/NPAGE, changing the behaviour to page_up/page_down to match.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
astyled astyled PR, label is assigned by github actions BasicBuildPassed This PR builds correctly, label assigned by github actions [C++] Changes (can be) made in C++. Previously named `Code` Info / User Interface Game - player communication, menus, etc. json-styled JSON lint passed, label assigned by github actions
Projects
None yet
3 participants