Some buttons are enabled when they shouldn't and show the wrong caption when no mod selected.
Details
- Reviewers
- None
- Commits
- rP23506: Fix button caption and enable in modmod
- Trac Tickets
- #5155
agree on the choosen context
Diff Detail
- Repository
- rP 0 A.D. Public Repository
- Branch
- /ps/trunk
- Lint
Lint OK - Unit
No Unit Test Coverage - Build Status
Buildable 7298 Build 11884: Vulcan Build Jenkins Build 11883: arc lint + arc unit
Event Timeline
Build failure - The Moirai have given mortals hearts that can endure.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | 367| 367| | 368| 368| g_ModsEnabled.sort((folder1, folder2) => | 369| 369| dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 : | 370| |- dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | | 370|+ dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | 371| 371| | 372| 372| displayModList("modsEnabledList", g_ModsEnabled); | 373| 373| }
Link to build: https://jenkins.wildfiregames.com/job/differential/726/display/redirect
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
372 | Since there are likely no other Enable/Disable strings in the modselecton screen and since the modselection screen is the only page in the transifex resource, it might work without the context too. But not wrong, good to add it in advance. |
While proofreading the modmod files for completeness, I found that the default caption of this button is inconsistent. It is shown when the button is disabled by default, but the caption remains Enabled if there is no mod selected. It would be more consistent to always have the same state if there is no mod selected.
I don't see any other i18n violation in these two files.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | 367| 367| | 368| 368| g_ModsEnabled.sort((folder1, folder2) => | 369| 369| dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 : | 370| |- dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | | 370|+ dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | 371| 371| | 372| 372| displayModList("modsEnabledList", g_ModsEnabled); | 373| 373| }
Link to build: https://jenkins.wildfiregames.com/job/differential/730/display/redirect
Fix the inconsistency noticed by elexis for the Enable/Disable button, the website button and the arrows.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Default... Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | 351| 351| | 352| 352| g_ModsEnabled.sort((folder1, folder2) => | 353| 353| dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 : | 354| |- dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | | 354|+ dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | 355| 355| | 356| 356| displayModList("modsEnabledList", g_ModsEnabled); | 357| 357| }
Link to build: https://jenkins.wildfiregames.com/job/differential/782/
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 |
|
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 | What else should the caption of the button be if no mod has been selected? |
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 | Why not just KISS and use "Enable" or "Disable" with the button being disabled if nothing is selected? It's accurate because the user cannot enable or disable if nothing is selected. It's the same behavior with all our other buttons that are disabled conditionally, no? |
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 |
I don't think we have much buttons that show different captions and are sometimes disabled... |
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 | It would be the first button that shows a different caption depending on it's enabled property. |
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
379 | I think it's nicer with the third string, and I mean it doesn't really hurt doing it, right? |
Ah sorry for missing this patch, I committed the simplest fix for #5155, which was just adding the translate call.
So this patch needs a rebase and a new description if the logic change for this button is agreed upon.
Successful build - Chance fights ever on the side of the prudent.
Linter detected issues: Executing section Source... Executing section JS... | | [NORMAL] ESLintBear (indent): | | Expected indentation of 3 tabs but found 2. |----| | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/mod/gui/modmod/modmod.js | 351| 351| | 352| 352| g_ModsEnabled.sort((folder1, folder2) => | 353| 353| dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 : | 354| |- dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | | 354|+ dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0); | 355| 355| | 356| 356| displayModList("modsEnabledList", g_ModsEnabled); | 357| 357| } Executing section cli...
Link to build: https://jenkins.wildfiregames.com/job/differential/1283/display/redirect
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
354 | Could fix the warning here while at it :) |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/differential/1284/display/redirect
Rebase, Only two states for Enable/Disable button, delete caption of button in xml file
binaries/data/mods/mod/gui/modmod/modmod.js | ||
---|---|---|
354 | Good catch. |
Successful build - Chance fights ever on the side of the prudent.
Link to build: https://jenkins.wildfiregames.com/job/docker-differential/1774/display/redirect