Skip to content

Move main.js to modules and help tab #2430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
mikeller merged 4 commits into from
Mar 16, 2021
Merged

Move main.js to modules and help tab #2430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
085cdc4
chore: move injecte_methods to modules
chmelevskij Mar 1, 2021
6c8dd5d
feat: migrate main to modules
chmelevskij Mar 1, 2021
68b3964
feat: move help tab to modules
chmelevskij Mar 1, 2021
d8be3dd
feat: migrate landing to modules
chmelevskij Mar 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion gulpfile.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -320,7 +320,8 @@ function dist_rollup() {
'components/init': 'src/components/init.js',
'js/main_cordova': 'src/js/main_cordova.js',
'js/utils/common': 'src/js/utils/common.js',
'js/tabs/logging': 'src/js/tabs/logging.js'
'js/tabs/logging': 'src/js/tabs/logging.js',
'js/main': 'src/js/main.js',
},
plugins: [
alias({
Expand Down
1 change: 1 addition & 0 deletions src/components/init.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// in the future it should be pure. This means it should
// explicitly export things used by other parts of the app.
import '../js/localization.js';
import '../js/injected_methods';
import i18next from 'i18next';
import Vue from "vue";
import vueI18n from "./vueI18n.js";
Expand Down
24 changes: 18 additions & 6 deletions src/js/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
'use strict';

window.googleAnalytics = analytics;
window.analytics = null;
import { i18n } from './localization';

$(document).ready(function () {

Expand Down Expand Up @@ -298,7 +295,7 @@ function startProcess() {

switch (tab) {
case 'landing':
TABS.landing.initialize(content_ready);
import('./tabs/landing').then(({ landing }) => landing.initialize(content_ready));
break;
case 'changelog':
TABS.staticTab.initialize('changelog', content_ready);
Expand All @@ -313,7 +310,7 @@ function startProcess() {
TABS.firmware_flasher.initialize(content_ready);
break;
case 'help':
TABS.help.initialize(content_ready);
import('./tabs/help').then(({ help }) => help.initialize(content_ready));
Copy link
Member Author

@chmelevskij chmelevskij Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Idea is to move the rest of the tabs to this usage. This way every tab and it's dependencies will be migrated one by one. Eventually leaving single main.js as entry point and removing the global variables.

Copy link
Member

@McGiverGim McGiverGim Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok. It seems a little strange to me but I don't have experience enough to know if this is the best way or not.

Copy link
Member Author

@chmelevskij chmelevskij Mar 8, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tried few different approaches. All of them are hard, because there was no clear separation on how to do it incrementally. This approach with TABS being the separation point is the most straightforwards.

we could definitely change the way tabs are exported and handled. That will be the next step. Didn't wan't to change a that because there are so many tabs already, and testing/reviewing a lot of changes will be hard enough already, there are a lot of other small things which have to be resolved. Like circular dependencies and random global variables.

Copy link
Member

@mikeller mikeller Mar 15, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The approach to do the conversion step-by-step in this way seems reasonable.

What is the reason you chose to import the tabs here, instead of importing all of them at the top of the file, and just initialising them here?

break;
case 'auxiliary':
TABS.auxiliary.initialize(content_ready);
Expand Down Expand Up @@ -545,6 +542,7 @@ function setDarkTheme(enabled) {
});
}


function checkForConfiguratorUpdates() {
const releaseChecker = new ReleaseChecker('configurator', 'https://api.github.com/repos/betaflight/betaflight-configurator/releases');

Expand Down Expand Up @@ -712,3 +710,17 @@ function showDialogDynFiltersChange() {
});
}
}

// TODO: all of these are used as globals in other parts.
// once moved to modules extract to own module.
window.showDialogDynFiltersChange = showDialogDynFiltersChange;
window.googleAnalytics = analytics;
window.analytics = null;
window.showErrorDialog = showErrorDialog;
window.generateFilename = generateFilename;
window.updateTabList = updateTabList;
window.isExpertModeEnabled = isExpertModeEnabled;
window.checkForConfiguratorUpdates = checkForConfiguratorUpdates;
window.setDarkTheme = setDarkTheme;
window.appReady = appReady;
window.checkSetupAnalytics = checkSetupAnalytics;
13 changes: 8 additions & 5 deletions src/js/tabs/help.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

TABS.help = {};
TABS.help.initialize = function (callback) {
const help = {};
help.initialize = function (callback) {

if (GUI.active_tab != 'help') {
GUI.active_tab = 'help';
Expand All @@ -14,6 +12,11 @@ TABS.help.initialize = function (callback) {
});
};

TABS.help.cleanup = function (callback) {
help.cleanup = function (callback) {
if (callback) callback();
};

// TODO: remove when modules are in place
window.TABS.help = help;

export { help };
14 changes: 9 additions & 5 deletions src/js/tabs/landing.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
'use strict';

TABS.landing = {};
TABS.landing.initialize = function (callback) {
const landing = {};
landing.initialize = function (callback) {

if (GUI.active_tab != 'landing') {
GUI.active_tab = 'landing';
Expand Down Expand Up @@ -49,6 +47,12 @@ TABS.landing.initialize = function (callback) {

};

TABS.landing.cleanup = function (callback) {
landing.cleanup = function (callback) {
if (callback) callback();
};

// TODO: remove after all is using modules
window.TABS.landing = landing;
export {
landing
};
6 changes: 1 addition & 5 deletions src/main.html
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,13 +75,11 @@
<!-- TODO: remove when using modules fully -->
<script type="module" src="./js/utils/css.js"></script>
<script type="text/javascript" src="./js/utils/window_watchers.js"></script>
<script type="text/javascript" src="./js/utils/CommonUtils.js"></script>
<script type="text/javascript" src="./js/utils/VtxDeviceStatus/VtxDeviceStatusFactory.js"></script>
<script type="text/javascript" src="./js/utils/VtxDeviceStatus/VtxDeviceStatus.js"></script>
<script type="text/javascript" src="./js/utils/VtxDeviceStatus/TrampDeviceStatus.js"></script>
<script type="text/javascript" src="./js/utils/VtxDeviceStatus/SmartAudioDeviceStatus.js"></script>
<script type="text/javascript" src="./js/utils/VtxDeviceStatus/Rtc6705DeviceStatus.js"></script>
<script type="text/javascript" src="./js/injected_methods.js"></script>
<script type="text/javascript" src="./js/ConfigStorage.js"></script>
<script type="text/javascript" src="./js/data_storage.js"></script>
<script type="text/javascript" src="./js/fc.js"></script>
Expand Down Expand Up @@ -110,13 +108,11 @@
<script type="text/javascript" src="./js/jenkins_loader.js"></script>
<script type="text/javascript" src="./js/Analytics.js"></script>
<script type="text/javascript" src="./js/GitHubApi.js"></script>
<script type="text/javascript" src="./js/main.js"></script>
<script type="module" src="./js/main.js"></script>
<script type="text/javascript" src="./js/Clipboard.js"></script>
<script type="text/javascript" src="./js/tabs/static_tab.js"></script>
<script type="text/javascript" src="./js/tabs/landing.js"></script>
<script type="text/javascript" src="./js/tabs/setup.js"></script>
<script type="text/javascript" src="./js/tabs/setup_osd.js"></script>
<script type="text/javascript" src="./js/tabs/help.js"></script>
<script type="text/javascript" src="./js/tabs/ports.js"></script>
<script type="text/javascript" src="./js/tabs/configuration.js"></script>
<script type="text/javascript" src="./js/tabs/pid_tuning.js"></script>
Expand Down