Skip to content

Formspec: change tabs with ctrl+tab#16167

Merged
SmallJoker merged 1 commit into
luanti-org:masterfrom
siliconsniffer:tab-ctrl
Jun 7, 2025
Merged

Formspec: change tabs with ctrl+tab#16167
SmallJoker merged 1 commit into
luanti-org:masterfrom
siliconsniffer:tab-ctrl

Conversation

@siliconsniffer

@siliconsniffer siliconsniffer commented May 23, 2025

Copy link
Copy Markdown
Contributor

This PR adds the ability to switch to the next/previous tab using ctrl(+shift) +tab.

Note: I have used AI during development.
  • Goal of the PR
    Make it easier/quicker to go to the next/previous tab using keyboard controls.

  • How does the PR work?
    It modifies the GUIFormSpecMenu's OnEvent method to cycle through tabs using the above mentioned keyboard shortcut.

    There are also a few event handling refinements:

    • Updated GUIFormSpecMenu's preprocessEvent method to prevent Ctrl+Tab and Return keys from being incorrectly consumed by child elements like checkboxes, tables, and list boxes.
    • Modified GUITable to pass key events with the Control modifier to its parent, ensuring consistent event propagation.
    • Adjusted GUITable's OnEvent method to no longer consume the Space key, allowing it to pass to the parent element instead.
  • Does it resolve any reported issue?
    Not any that I am aware of.

  • Does this relate to a goal in the roadmap?
    Hopefully 2.3.

  • If not a bug fix, why is this PR needed? What usecases does it solve?
    It makes it easier and quicker to go to the next or previous tab.

To do

This PR is ready for Review.

  • Add demo video?
  • Discuss changes regarding event handling but unrelated to the tab cycling

Preview

2025-05-26.20-05-54.mp4

How to test

Open Luanti → try switching forth and back using the keyboard shortcut.

@Zughy Zughy added Roadmap The change matches an item on the current roadmap @ Mainmenu / Settingsmenu @ Client / Controls / Input Formspec UI/UX Feature ✨ PRs that add or enhance a feature labels May 23, 2025
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
@SmallJoker

Copy link
Copy Markdown
Member

Quick review. I like this feature and also wonder why no-one else has yet come up with this...
Thank you.

@siliconsniffer

siliconsniffer commented May 26, 2025

Copy link
Copy Markdown
Contributor Author

Thanks for the review and the kind words. I was wondering the same!
It's really a small but nice QoL improvement and somewhat an "industry standard" already.

Edit: The only issue I found can be seen e.g. in devtest in the "Tabs" tab, where instead of the test_formspec tabs it will focus the tabs inside of that page. Similar behavior happening in the "Real Coord" tab.

@SmallJoker SmallJoker left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

The tab control is not always selected correctly, but generally there's at most one, which makes it tolerable.
I tried to fix that by checking the parent of the focused element - but tab controls are apparently only used for visuals and not to actually contain elements (as it would be intended by the Irrlicht API). I suppose the current solution must suffice.

Comment thread src/gui/guiFormSpecMenu.cpp
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
Comment thread src/gui/guiTable.cpp Outdated
Comment thread src/gui/guiTable.cpp Outdated
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
@sfan5
sfan5 requested a review from grorp June 1, 2025 12:54
@siliconsniffer

Copy link
Copy Markdown
Contributor Author

I tried to fix that by checking the parent of the focused element - but tab controls are apparently only used for visuals and not to actually contain elements (as it would be intended by the Irrlicht API). I suppose the current solution must suffice.

Isn't that worth opening an issue for (maybe even just to document it)?

@SmallJoker

Copy link
Copy Markdown
Member

Isn't that worth opening an issue for [...]?

It's an implementation detail. Formspecs could be made more responsive but I wonder whether that would justify additional traffic (to send all tab contents in the same formspec),.
One could perhaps open an issue about that, but I honestly don't care, thus no action from my side.

Comment thread src/gui/guiFormSpecMenu.cpp
Comment thread src/gui/guiTable.cpp
Comment thread src/gui/guiFormSpecMenu.cpp Outdated
@grorp grorp added the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jun 1, 2025
@grorp grorp removed the Action / change needed Code still needs changes (PR) / more information requested (Issues) label Jun 5, 2025
@SmallJoker
SmallJoker merged commit 4454d71 into luanti-org:master Jun 7, 2025
17 checks passed
@siliconsniffer
siliconsniffer deleted the tab-ctrl branch June 18, 2025 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants