Code gardening: UiController.js

[cake-pie]

Issue Description

Doing some cleanup in UiController.js.
Filing to get an issue number for commits, branch name and eventually PR.

• fixing incorrect JSDoc comments for properties (copy-and-pasted and not changed accordingly)
• add and document missing properties trafficRateController, $toggleAirspace to constructor + destructor
• use properties to access footer menu buttons in various onToggleXyz handlers instead of making unecessary jQuery objects
• add property this.$log for use in ui_log
• formatting/indentation consistency
• use data attribute to lookup row for current airport in airport switch dialog (instead of abusing class)
• alphabetize assignments to jQuery objects in: constructor, init, enable, disable, destroy
• group / alphabetize methods
• modify a few footer menu button click handlers so that analytics will have saner output. (originally reporting "false" for open/on and "true" for close/off)
• report both opening and closing of tutorial for analytics

No change to code logic or function.
Okay that turned out to be a lie, because of the last two items in the above list.

[cake-pie]

@n8rzz thought I should ask beforehand to avoid having to revert or change in code review:
How should I re-order this block of code? (from UiController init() method)

openscope/src/assets/scripts/client/ui/UiController.js

Lines 298 to 320 in 67f23ec

	this.$airportDialog = this.$element.find(SELECTORS.DOM_SELECTORS.AIRPORT_SWITCH);
	this.$airportDialogBody = this.$airportDialog.find(SELECTORS.DOM_SELECTORS.DIALOG_BODY);
	this.$airportGuideDialog = this.$element.find(SELECTORS.DOM_SELECTORS.AIRPORT_GUIDE_CONTAINER);
	this.$changelogDialog = this.$element.find(SELECTORS.DOM_SELECTORS.CHANGELOG_CONTAINER);
	this.$tutorialDialog = this.$element.find(SELECTORS.DOM_SELECTORS.TUTORIAL);
	this.$fastForwards = this.$element.find(SELECTORS.DOM_SELECTORS.FAST_FORWARDS);
	this.$githubLinkElement = this.$element.find(SELECTORS.DOM_SELECTORS.GITHUB_EXTERNAL_LINK);
	this.$pausedImg = this.$element.find(`${SELECTORS.DOM_SELECTORS.PAUSED} img`);
	this.$switchAirport = this.$element.find(SELECTORS.DOM_SELECTORS.SWITCH_AIRPORT);
	this.$toggleAirportGuide = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_AIRPORT_GUIDE);
	this.$toggleAirspace = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_AIRSPACE);
	this.$toggleChangelog = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_CHANGELOG);
	this.$toggleLabels = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_LABELS);
	this.$toggleOptions = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_OPTIONS);
	this.$togglePause = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_PAUSE);
	this.$toggleRestrictedAreas = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_RESTRICTED_AREAS);
	this.$toggleSids = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_SIDS);
	this.$toggleSpeech = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_SPEECH);
	this.$toggleStars = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_STARS);
	this.$toggleTerrain = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_TERRAIN);
	this.$toggleTutorial = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_TUTORIAL);
	this.$toggleVideoMap = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_VIDEO_MAP);
	this.$toggleTraffic = this.$element.find(SELECTORS.DOM_SELECTORS.TOGGLE_TRAFFIC);

I could:
a) enforce alphabetical order of whole block of code (move noncompliant L302, L320)
b) separate sections for elements related to dialogs and footer menu buttons, both alphabetical order
c) separate sections: dialogs alphabetically, footer buttons according to visual/DOM ordering

Will then apply the same consistent ordering throughout (constructor, enable, disable)

[n8rzz]

Yes please!

…

On Jan 30, 2021, at 07:34, cake>pie ***@***.***> wrote:  @n8rzz thought I should ask beforehand to avoid having to revert or change in code review: How should I re-order this block of code? (from UiController init() method) https://github.com/openscope/openscope/blob/67f23ecfa27e73eb698ee7fe7c008ac03fdb8d3e/src/assets/scripts/client/ui/UiController.js#L298-L320 I could: a) enforce alphabetical order of whole block of code (move noncompliant L302, L320) b) separate sections for elements related to dialogs and footer menu buttons, both alphabetical order c) separate sections: dialogs alphabetically, footer buttons according to visual/DOM ordering Will then apply the same consistent ordering throughout (constructor, enable, disable) — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or unsubscribe.

[cake-pie]

Please choose one of:

a) enforce alphabetical order of whole block of code (move noncompliant L302, L320)
b) split into separate sections for elements related to dialogs and footer menu buttons, both alphabetical order
c) separate sections: dialogs alphabetically, footer buttons according to visual/DOM ordering

[n8rzz]

...and thats what I get for replying by email. I am ashamed.

I'd go for a. And, as you do, keep an eye out for abstractions. There are multiple dialogs that we use, should there be a DialogController that worries about each with the UiController acting simply as traffic cop?

I wrote a lot of this 5 or 6 years ago now, a lot of it makes me throw up a bit. Some of it isn't bad. All of it will benefit from fresh, critical eyes.

[cake-pie]

There are multiple dialogs that we use, should there be a DialogController that worries about each with the UiController acting simply as traffic cop?

I think #1325 aims to do that?