Page MenuHomeWildfire Games

Make modmod looking nicer
ClosedPublic

Authored by Imarok on Mar 24 2018, 11:33 AM.

Details

Summary

Make modmod easier to use and look better. Things changed:

Only enable buttons when they are usable.
Only one "Visit Website" button.
Merge the Enable and the Disable button.
By that, make only one mod (of both lists) selectable at the same time.
Use arrows instead of up/down buttons.
Some images (Code is coming soon):




Test Plan

Open the Mod Selection screen.
Select some mods.
Enable & Disable them move mods up/down.

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

Imarok created this revision.Mar 24 2018, 11:33 AM
Imarok retitled this revision from [modmod] Only enable buttons when usable to [modmod] Only enable buttons when they are usable.Mar 24 2018, 11:34 AM
Imarok edited the summary of this revision. (Show Details)
Vulcan added a subscriber: Vulcan.Mar 24 2018, 11:40 AM

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
| 341| 341| 
| 342| 342| 	g_ModsEnabled.sort((folder1, folder2) =>
| 343| 343| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 344|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 344|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 345| 345| 
| 346| 346| 	displayModList("modsEnabledList", g_ModsEnabled);
| 347| 347| }

Link to build: https://jenkins.wildfiregames.com/job/differential/306/display/redirect

Imarok retitled this revision from [modmod] Only enable buttons when they are usable to [modmod] gui changes.Apr 6 2018, 1:20 AM
Imarok edited the summary of this revision. (Show Details)
Imarok edited the test plan for this revision. (Show Details)
Imarok added a reviewer: elexis.
Imarok retitled this revision from [modmod] gui changes to Make modmod looking nicer.Apr 6 2018, 1:25 AM
Imarok edited the summary of this revision. (Show Details)
elexis added a comment.Apr 6 2018, 4:13 AM
  • arrows too large IMO. Also if it was up to me I'd colorize them similar to the red buttons.
  • any reason to not have the buttons on one line? Current layout looks like it would bug or appear oddly for long descriptions also.
  • Reset Filters button seems unneeded now
  • See rP20552 for some arguments about Disabled being a bad name that I don't share
  • Max-one-selection approach probably ok

Could you remove the useless space on the top?

Notes:

  • I agree with @elexis, too big buttons. And we need to make them look consistent with other GUI elements.
  • Why red buttons are on different lines? It looks strange, and costs a useful space.
  • It'd be good to add a description/tooltip/help, why do we need these up/down buttons for a user.
In D1412#58534, @elexis wrote:
  • arrows too large IMO. Also if it was up to me I'd colorize them similar to the red buttons.

I have no problem making them smaller. Wrt colorization: I'd do that too, but no idea how without drawing something (which I cannot do with an appropriate quality)

  • any reason to not have the buttons on one line? Current layout looks like it would bug or appear oddly for long descriptions also.

Which buttons are you talking about? (If you mean Enable/Disable and Visit Website, that is intended: First line are the selected-mod-specific buttons, second line the "global" buttons)

  • Reset Filters button seems unneeded now

True.

  • See rP20552 for some arguments about Disabled being a bad name that I don't share

Wanna propose a better naming?

Could you remove the useless space on the top?

I thought that too first, but this is the space were messages are displayed. E.g.:

Notes:

  • I agree with @elexis, too big buttons. And we need to make them look consistent with other GUI elements.
  • Why red buttons are on different lines? It looks strange, and costs a useful space.

see my comment above.

  • It'd be good to add a description/tooltip/help, why do we need these up/down buttons for a user.

True, any text to propose?

True, any text to propose?

Ideally it'd specify a feature description, use case and example.
Maybe "Change the order in which mods are launched. This should match the mods dependencies." What dependencies are is at least seen in the table.
The example could be presented in case of launching mods that don't match the dependency.

vladislavbelov added a comment.EditedApr 7 2018, 10:36 AM
  • any reason to not have the buttons on one line? Current layout looks like it would bug or appear oddly for long descriptions also.

Which buttons are you talking about? (If you mean Enable/Disable and Visit Website, that is intended: First line are the selected-mod-specific buttons, second line the "global" buttons)

Yes, it costs space and not splitted logically. For user it looks strange. So, I suggest to make them on the one line with different space between groups (or another visual delimiter). Or move status and description to the bottom and split them by a horizontal line. Then you're able to remove useless empty space on the top.

  • any reason to not have the buttons on one line? Current layout looks like it would bug or appear oddly for long descriptions also.

Which buttons are you talking about? (If you mean Enable/Disable and Visit Website, that is intended: First line are the selected-mod-specific buttons, second line the "global" buttons)

Yes, it costs space and not splitted logically. For user it looks strange. So, I suggest to make them on the one line with different space between groups (or another visual delimiter). Or move status and description to the bottom and split them by a horizontal line. Then you're able to remove useless empty space on the top.

Even if there is a fourth button added to the button row? (Might happen at some time...)

vladislavbelov added a comment.EditedApr 7 2018, 10:43 AM
In D1412#58646, @Imarok wrote:
  • any reason to not have the buttons on one line? Current layout looks like it would bug or appear oddly for long descriptions also.

Which buttons are you talking about? (If you mean Enable/Disable and Visit Website, that is intended: First line are the selected-mod-specific buttons, second line the "global" buttons)

Yes, it costs space and not splitted logically. For user it looks strange. So, I suggest to make them on the one line with different space between groups (or another visual delimiter). Or move status and description to the bottom and split them by a horizontal line. Then you're able to remove useless empty space on the top.

Even if there is a fourth button added to the button row? (Might happen at some time...)

Yes, we don't have these not added buttons (only 1), but we have the release. And it's too much reserve for new buttons.

Imarok planned changes to this revision.Apr 8 2018, 4:43 PM
This comment was removed by Imarok.
Imarok updated this revision to Diff 6372.Apr 11 2018, 7:51 PM

Proposed changes

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
| 334| 334| 
| 335| 335| 	g_ModsEnabled.sort((folder1, folder2) =>
| 336| 336| 		dependencies[folder1].indexOf(g_Mods[folder2].name) != -1 ? 1 :
| 337|    |-		dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
|    | 337|+			dependencies[folder2].indexOf(g_Mods[folder1].name) != -1 ? -1 : 0);
| 338| 338| 
| 339| 339| 	displayModList("modsEnabledList", g_ModsEnabled);
| 340| 340| }
|    | [NORMAL] ESLintBear (no-multi-spaces):
|    | Multiple spaces found before 'Engine'.
|----|    | /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
| 349| 349| 	{
| 350| 350| 		otherListObject.selected = -1;
| 351| 351| 		Engine.GetGUIObjectByName("visitWebButton").enabled = true;
| 352|    |-		let disEnableButton =  Engine.GetGUIObjectByName("disEnableButton")
|    | 352|+		let disEnableButton = Engine.GetGUIObjectByName("disEnableButton")
| 353| 353| 		disEnableButton.caption = listObjectName == "modsDisabledList" ? "Enable" : "Disable";
| 354| 354| 		disEnableButton.enabled = true;
| 355| 355| 		disEnableButton.onPress = listObjectName == "modsDisabledList" ? enableMod : disableMod;
|    | [NORMAL] ESLintBear (semi):
|    | Missing semicolon.
|----|    | /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
| 349| 349| 	{
| 350| 350| 		otherListObject.selected = -1;
| 351| 351| 		Engine.GetGUIObjectByName("visitWebButton").enabled = true;
| 352|    |-		let disEnableButton =  Engine.GetGUIObjectByName("disEnableButton")
|    | 352|+		let disEnableButton =  Engine.GetGUIObjectByName("disEnableButton");
| 353| 353| 		disEnableButton.caption = listObjectName == "modsDisabledList" ? "Enable" : "Disable";
| 354| 354| 		disEnableButton.enabled = true;
| 355| 355| 		disEnableButton.onPress = listObjectName == "modsDisabledList" ? enableMod : disableMod;

binaries/data/mods/mod/gui/modmod/modmod.js
| 352| »   »   let·disEnableButton·=··Engine.GetGUIObjectByName("disEnableButton")
|    | [NORMAL] JSHintBear:
|    | Missing semicolon.

Link to build: https://jenkins.wildfiregames.com/job/differential/376/display/redirect

Itms accepted this revision.Apr 14 2018, 3:55 PM
Itms added a subscriber: Itms.

The result looks nice to me, the button size is correct now.

I find the button split logical and the fourth button is coming in D1029 anyway.

This revision is now accepted and ready to land.Apr 14 2018, 3:55 PM
This revision was automatically updated to reflect the committed changes.