Split prelobby into multiple pages and require terms acceptance prior to login
ClosedPublic
Actions

Authored by elexis on Jun 9 2018, 3:02 AM.

Details

Reviewers: None
Commits: rP21847: Rewrite the prelobby pages and add the Terms of Service, Terms of Use and the…
Trac Tickets: #5218

Summary

Since there should be a way to accept changed terms and conditions after already having registered an account,
the login page needs to receive the Terms Of Service button, Terms of Use button, acceptance label, acceptance checkbox and feedback label too.

Since the current code mixes three different GUI pages into one without actually removing the duplication, this patch splits and simplifies the code
rather than making the mess worse.

The patch reuses all existing strings and keeps all strings within the same translation resource so as not to add untranslated strings in the alpha 23 rerelease.

New login page design:

Unchanged registration page design:

Test Plan

Investigate messages.json. Read the previous code and check if all features and edge cases are passed on. Become convinced that the login and register page should not be unified for as they are intrinsically different and should be separately modifiable.

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

elexis created this revision.Jun 9 2018, 3:02 AM

Not done yet

elexis updated the Trac tickets for this revision.Jun 9 2018, 3:05 AM

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/prelobby/register/register.js
|  28| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/prelobby/login/login.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/prelobby/login/login.js
|  99|  99| 	let checkbox = Engine.GetGUIObjectByName("rememberPassword");
| 100| 100| 	let enabled = Engine.ConfigDB_GetValue("user", "lobby.rememberpassword") == "true";
| 101| 101| 	if (!checkbox.checked && enabled && Engine.ConfigDB_GetValue("user", "lobby.password"))
| 102|    |-	{
|    | 102|+	
| 103| 103| 		messageBox(
| 104| 104| 			360, 160,
| 105| 105| 			translate("Are you sure you want to delete the password after connecting?"),
| 110| 110| 				() => { saveSettingAndWriteToUserConfig("lobby.rememberpassword", String(!enabled)); }
| 111| 111| 			]
| 112| 112| 		);
| 113|    |-	}
|    | 113|+	
| 114| 114| 	else
| 115| 115| 		saveSettingAndWriteToUserConfig("lobby.rememberpassword", String(!enabled));
| 116| 116| }

binaries/data/mods/public/gui/prelobby/login/login.js
|  52| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.

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

Harbormaster completed remote builds in B6235: Diff 6741.Jun 9 2018, 3:08 AM

Added few missing code, tested most if not all cases.
Even more minimal and pleasent JS code.
Display remember-pw thing also when registering.
Tried to replace the buggy display of only the first 10 characters, ended up removing it, considering adding it again.
Split futher, seemingly elegant XML only approach to have different locations per object without duplicating objects.
Fix rP20064 bug.

Successful build - Chance fights ever on the side of the prudent.

Linter detected issues:
Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/prelobby/common/feedback/feedback.js
|  15| »   while·(true)
|    | [NORMAL] ESLintBear (no-constant-condition):
|    | Unexpected constant condition.
|    | [NORMAL] ESLintBear (curly):
|    | Unnecessary { after 'if' condition.
|----|    | /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/prelobby/common/credentials/credentials.js
|    |++++| /mnt/data/jenkins-phabricator/workspace/differential/binaries/data/mods/public/gui/prelobby/common/credentials/credentials.js
|  45|  45| 	let checkbox = Engine.GetGUIObjectByName("rememberPassword");
|  46|  46| 	let enabled = Engine.ConfigDB_GetValue("user", "lobby.rememberpassword") == "true";
|  47|  47| 	if (!checkbox.checked && enabled && Engine.ConfigDB_GetValue("user", "lobby.password"))
|  48|    |-	{
|    |  48|+	
|  49|  49| 		messageBox(
|  50|  50| 			360, 160,
|  51|  51| 			translate("Are you sure you want to delete the password after connecting?"),
|  56|  56| 				() => { saveSettingAndWriteToUserConfig("lobby.rememberpassword", String(!enabled)); }
|  57|  57| 			]
|  58|  58| 		);
|  59|    |-	}
|    |  59|+	
|  60|  60| 	else
|  61|  61| 		saveSettingAndWriteToUserConfig("lobby.rememberpassword", String(!enabled));
|  62|  62| }

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

Harbormaster completed remote builds in B6239: Diff 6748.Jun 11 2018, 1:32 AM

elexis edited the summary of this revision. (Show Details)Jun 11 2018, 2:18 PM

This revision was not accepted when it landed; it landed in state Needs Review.Jun 11 2018, 6:01 PM

Closed by commit rP21847: Rewrite the prelobby pages and add the Terms of Service, Terms of Use and the… (authored by elexis). · Explain Why

This revision was automatically updated to reflect the committed changes.

Owners added a subscriber: Restricted Owners Package.Jun 11 2018, 6:01 PM

elexis mentioned this in D1575: Persist lobby terms agreement checkbox if the terms didn't change.Jun 12 2018, 12:33 PM

elexis mentioned this in rP21887: Refactor lobby terms and conditions UI from rP21847 / D1568 to a new terms….Sep 14 2018, 5:15 PM