Page MenuHomeWildfire Games

On Cancel Hide Tab Content On New Gamesetup UI
ClosedPublic

Authored by ffffffff on Jan 22 2018, 7:23 AM.

Details

Summary

hit cancel to hide (escape)

Test Plan

hit cancel (escape)

Diff Detail

Repository
rP 0 A.D. Public Repository
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

bb added a comment.Jan 22 2018, 2:39 PM

This has the same problem as described in D1244 (hit esc quickly after opening the gamesetup), but it should be fixed by checking for onSelectTab in selectPanel, further patch looks good

bb added inline comments.Jan 27 2018, 11:58 PM
binaries/data/mods/public/gui/common/tab_buttons.js
70–72 ↗(On Diff #5525)

imo only the onSelectTab call doesn't have to be executed, but the setting of g_TabCategory and setting some sprites can be executed, when pressing esc before the init, the tabs will be hidden. So the check can move to old L75, this also allows not setting the function, if that is required somewhere

bb added a comment.Feb 9 2018, 10:49 PM

A hotkey on the button would be nice

bb added inline comments.Apr 16 2018, 1:05 PM
binaries/data/mods/public/gui/gamesetup/gamesetup.xml
269–274 ↗(On Diff #5536)

inline the hotkey stuff here (like the way it is done for the lobby button)

275 ↗(On Diff #5536)

also the tooltip should be merged with the already existing tooltip: "Return to lobby."

bb accepted this revision.Apr 23 2018, 3:16 PM

I misread in last comment, patch actually correct. A hotkey string should be added after the release

This revision is now accepted and ready to land.Apr 23 2018, 3:16 PM
elexis added inline comments.Apr 23 2018, 3:42 PM
binaries/data/mods/public/gui/gamesetup/gamesetup.xml
10 ↗(On Diff #5536)

If there were more collapsible elements, one might consider moving that to the tabs UI and possibly creating a custom hotkey to distinguish closing of the options dialog.
Just because we want to assign Escape by default doesn't imply that we have to use cancel (I guess that term is used a bit broadly as it also allows closing of pages after saving or this thing here which isnt cancelling anything either)
But should be good enough for now.

This revision was automatically updated to reflect the committed changes.
Owners added a subscriber: Restricted Owners Package.Apr 23 2018, 4:08 PM