Give the tabs a hotkey tooltip, this needs several hotkeys colored in the same string, thus extending support for that.
Details
Details
- Reviewers
- None
Trigger all changed hotkeys
Diff Detail
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 4209 Build 7403: Vulcan Build (Windows) Jenkins Build 7402: Vulcan Build Jenkins Build 7401: arc lint + arc unit
Event Timeline
| binaries/data/mods/public/gui/common/color.js | ||
|---|---|---|
| 145–146 | adding that 0 everywhere and the arrays are a bit meh, but holding support for without that even more so imo | |
| binaries/data/mods/public/gui/common/tab_buttons.js | ||
| 46 | Looks a bit ugly when no tab specific hotkey is set, can be solved by setting those everywhere | |
| binaries/data/mods/public/gui/session/unit_actions.js | ||
| 1029–1030 | Do not merge since then the sting gets fully removed when the hotkey isn't set, which shouldn't happen | |
| binaries/data/mods/public/gui/common/tab_buttons.js | ||
|---|---|---|
| 47 | What if the user didn't assign a key to one of them? | |
Comment Actions
Successful build - Chance fights ever on the side of the prudent.
Updating workspaces... Build (release)... Build (debug)... Running release tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Running debug tests... Running cxxtest tests (308 tests)....................................................................................................................................................................................................................................................................................................................OK! Checking XML files...
Comment Actions
Don't really see the problem with this:
Press MouseWheelUp to select the previous tab.
Press MouseWheelDown to select the next tab.
since there are usually 1, sometimes 3, rarely 5 hotkeys per tooltip shown.
But indeed this looks nicer:
Press MouseWheelDown / MouseWheelUp to select the next / previous tab.
The former however seems a bit easier to compare (h1->k1, h2>k2 easier to read than k1/k2 assigned to h1/h2).
The 0/1 ugliness could be solved by passing { "hotkey_foo": "...", "hotkey_bar": ... }.
Then you don't have to change all of the strings either.
Every assigned key should be displayed.
Displaying "Press Unused / MouseWheelUp to select the previous / next tab" seems slightly wrong, since there is no unused key.
Comment Actions
Ah didn't know you made also a patch for this. (D1264)
How we get along with this?
D1264 got less changes in overall code by keeping the %(hotkey)s option as first hotkey object in colorizeHotkey function and then continue counting for further hotkey strings as %(hotkey1)s, %(hotkey2)s and so forth.
Further more it unifies summary tabs tooltip and tab buttons tooltip. But that can be done in a seperate patch or in this patch.
So i can either review this patch or one review mine and we take mine..
Ah and btw it seems @vladislavbelov can not finally review my patch D856 at the moment. Do you can have a look on it? Thx :)
| binaries/data/mods/public/gui/common/tab_buttons.js | ||
|---|---|---|
| 47 | return empty string by colorizeHotkeys instead of unassigned hotkey string | |