Allow the GUI to subscribe to text edit events to kill a combination of three workarounds in the options page
ClosedPublic
Actions

Authored by elexis on Sep 1 2017, 3:25 AM.

Details

Reviewers: None
Commits: rP20091: Allow the GUI to subscribe to text edit events to redeem three workarounds in…

Summary

The options page is inteded to update immediately if a user has changed any setting.
This works well for every control type except input fields which are governed by CInput.cpp.

There is only the event when the return key is pressed currently, so the options page does weird things to compensate.

It subscribes to the mouse leave event. However that is unrelated, because the mouse can have left the field before it was edited. Also the window can be closed without having the mouse leave the field.
Another workaround that was added is the registerChanges function that does the checking manually when closing the page for everything that the other two wrong subscriptions missed.

Instead, the GUI should just be informed about the text edit event.

Test Plan

Add <action on="Press">warn("edit");</action> to <object name="chatInput"... in gamesetup.xml to test that the event is sent if and only if the text was changed.

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.Sep 1 2017, 3:25 AM

Harbormaster completed remote builds in B2966: Diff 3442.Sep 1 2017, 3:25 AM

Rebase

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1945/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/options/options.js
|  97| »   »   onUpdate·=·function(key)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'minvalue' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxvalue' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'functionBody' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'minval' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxval' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 225| »   »   let·callbackFunction;
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 249| »   »   onUpdate·=·function(key,·callbackFunction)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 249| »   »   onUpdate·=·function(key,·callbackFunction)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 239| »   »   »   »   control.list·=·option.parameters.list.map(e·=>·translate(e.label));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/463/ for more details.

Harbormaster completed remote builds in B2966: Diff 3442.Sep 1 2017, 4:14 AM

Build is green

Updating workspaces.
Build (release)...
Build (debug)...
Running release tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Running debug tests...
Running cxxtest tests (306 tests)..................................................................................................................................................................................................................................................................................................................OK!
Checking XML files...

http://jenkins-master:8080/job/phabricator/1946/ for more details.

Executing section Default...
Executing section Source...
Executing section JS...

binaries/data/mods/public/gui/options/options.js
|  97| »   »   onUpdate·=·function(key)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'minvalue' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 142| »   »   onUpdate·=·function(key,·callbackFunction,·minvalue,·maxvalue)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxvalue' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'functionBody' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'minval' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 199| »   »   onUpdate·=·function(key,·functionBody,·minval,·maxval)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'maxval' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 225| »   »   let·callbackFunction;
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 249| »   »   onUpdate·=·function(key,·callbackFunction)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'key' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 249| »   »   onUpdate·=·function(key,·callbackFunction)
|    | [NORMAL] ESLintBear (no-shadow):
|    | 'callbackFunction' is already declared in the upper scope.

binaries/data/mods/public/gui/options/options.js
| 239| »   »   »   »   control.list·=·option.parameters.list.map(e·=>·translate(e.label));
|    | [NORMAL] JSHintBear:
|    | Don't make functions within a loop.
Executing section XML GUI...
Executing section Python...
Executing section Perl...

http://jenkins-master:8080/job/phabricator_lint/464/ for more details.

Harbormaster completed remote builds in B2968: Diff 3443.Sep 1 2017, 5:50 AM

Thanks Vladislav for testing the patch and discussing it in irc today.

One point discussed was whether to send an event on Tab or when the caption was changed from JS.
We agreed to not do that, because we don't want false positives events (i.e. avoid sending an event if the caption wasn't actually changed after a tab or caption = foo event).
It's easy for the JS side to subscribe to its own caption changes. Adding a comment in the code to clarify that to future readers of the code.

Closed by commit rP20091: Allow the GUI to subscribe to text edit events to redeem three workarounds in… (authored by elexis). · Explain WhySep 1 2017, 2:06 PM

This revision was automatically updated to reflect the committed changes.

elexis mentioned this in rP20550: Enable and disable mods more comfortably using the doubleclick event, refs….Nov 28 2017, 7:57 PM