Optimized loading of custom theme setting in design screens

no issue

We want to automatically show brand settings expanded in the design menu when the active theme has no custom theme settings, in order to do that without causing visual noise/jank we need to ensure that we have all the data we need up-front before the design menu is rendered.

- optimized `customThemeSettings` loading behaviour
  - `.load()` will now only perform a fetch if settings have not previously been loaded so it can be called without causing unnecessary waits
  - `.reload()` will force a clear+refetch of the settings - called by `themeManagement.activate()` after successfully changing a theme
- moved fetching of theme settings from the design menu constructor to the `settings.design` route's `model()` hook
  - means the app will wait for loading to finish before showing any of the design settings screen so we can guarantee the data we need is available
- moved update of preview html from the design menu constructor to the design settings route as it's a more appropriate place to find screen setup/loading behaviour
This commit is contained in:
Kevin Ansfield 2021-10-22 10:46:12 +01:00
parent e78e1d9cae
commit 505ae7493e
5 changed files with 28 additions and 11 deletions

View File

@ -1,7 +1,6 @@
import Component from '@glimmer/component';
import {action} from '@ember/object';
import {inject as service} from '@ember/service';
import {task} from 'ember-concurrency-decorators';
import {tracked} from '@glimmer/tracking';
export default class DesignMenuComponent extends Component {
@ -18,8 +17,6 @@ export default class DesignMenuComponent extends Component {
constructor() {
super(...arguments);
this.fetchThemeSettingsTask.perform();
this.themeManagement.updatePreviewHtmlTask.perform();
// fetch all themes in the background so we can show the active theme
this.store.findAll('theme');
@ -45,11 +42,6 @@ export default class DesignMenuComponent extends Component {
}
}
@task
*fetchThemeSettingsTask() {
yield this.customThemeSettings.load();
}
@action
transitionBackToIndex() {
if (this.router.currentRouteName !== 'settings.design.index') {

View File

@ -2,9 +2,11 @@ import AuthenticatedRoute from 'ghost-admin/routes/authenticated';
import {inject as service} from '@ember/service';
export default class SettingsDesignRoute extends AuthenticatedRoute {
@service customThemeSettings;
@service feature;
@service modals;
@service settings;
@service themeManagement;
@service ui;
beforeModel() {
@ -20,7 +22,16 @@ export default class SettingsDesignRoute extends AuthenticatedRoute {
}
model() {
return this.settings.reload();
// background refresh of preview
// not doing it on the 'index' route so that we don't reload going to/from the index,
// any actions performed on child routes that need a refresh should trigger it explicitly
this.themeManagement.updatePreviewHtmlTask.perform();
// wait for settings to be loaded - we need the data to be present before display
return Promise.all([
this.settings.reload(),
this.customThemeSettings.load()
]);
}
activate() {

View File

@ -2,7 +2,7 @@ import AuthenticatedRoute from 'ghost-admin/routes/authenticated';
import {action} from '@ember/object';
import {inject as service} from '@ember/service';
export default class SettingsDesignRoute extends AuthenticatedRoute {
export default class SettingsDesignIndexRoute extends AuthenticatedRoute {
@service customThemeSettings;
@service modals;
@service settings;

View File

@ -11,6 +11,8 @@ export default class CustomThemeSettingsServices extends Service {
@tracked settings = [];
@tracked settingGroups = [];
_hasLoaded = false;
KNOWN_GROUPS = [{
key: 'homepage',
name: 'Homepage',
@ -42,8 +44,18 @@ export default class CustomThemeSettingsServices extends Service {
return this.loadTask.perform();
}
reload() {
this._hasLoaded = false;
return this.loadTask.perform();
}
@task
*loadTask() {
if (this.hasLoaded) {
return this.settings;
}
// unload stored settings and re-load from API so they always match active theme
// run is required here, see https://github.com/emberjs/data/issues/5447#issuecomment-845672812
run(() => this.store.unloadAll('custom-theme-setting'));
@ -52,6 +64,8 @@ export default class CustomThemeSettingsServices extends Service {
this.settings = settings;
this.settingGroups = this._buildSettingGroups(settings);
this._hasLoaded = true;
return this.settings;
}

View File

@ -90,7 +90,7 @@ export default class ThemeManagementService extends Service {
const activatedTheme = yield theme.activate();
this.updatePreviewHtmlTask.perform();
this.customThemeSettings.load();
this.customThemeSettings.reload();
if (!options.skipErrors) {
const {warnings, errors} = activatedTheme;