Skip to content
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

[Feature] Add dynamic theme switching support #20705

Merged
merged 4 commits into from
Jan 23, 2025
Merged

[Feature] Add dynamic theme switching support #20705

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
4344997
feat(theme): dynamically register and set chart theme
Ovilia Jan 21, 2025
a14f462
fix(theme): update setTheme
Ovilia Jan 22, 2025
533206b
fix(theme): revert theme
Ovilia Jan 22, 2025
21c5ffc
fix(theme): update code according to review comments
Ovilia Jan 23, 2025
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
81 changes: 68 additions & 13 deletions src/core/echarts.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,10 @@ export interface ResizeOpts {
silent?: boolean // by default false.
};

export interface SetThemeOpts {
silent?: boolean;
}

interface PostIniter {
(chart: EChartsType): void
}
Expand Down Expand Up @@ -404,11 +408,6 @@ class ECharts extends Eventful<ECEventDefinition> {

opts = opts || {};

// Get theme by name
if (isString(theme)) {
theme = themeStorage[theme] as object;
}

this._dom = dom;

let defaultRenderer = 'canvas';
Expand Down Expand Up @@ -459,10 +458,7 @@ class ECharts extends Eventful<ECEventDefinition> {
// Expect 60 fps.
this._throttledZrFlush = throttle(bind(zr.flush, zr), 17);

theme = clone(theme);
theme && backwardCompat(theme as ECUnitOption, true);

this._theme = theme;
this._updateTheme(theme);

this._locale = createLocaleObject(opts.locale || SYSTEM_LANG);

Expand Down Expand Up @@ -693,10 +689,69 @@ class ECharts extends Eventful<ECEventDefinition> {
}

/**
* @deprecated
* Update theme with name or theme option and repaint the chart.
* @param theme Theme name or theme option.
* @param opts Optional settings
*/
private setTheme(): void {
deprecateLog('ECharts#setTheme() is DEPRECATED in ECharts 3.0');
setTheme(theme: string | ThemeOption, opts?: SetThemeOpts): void {
if (this[IN_MAIN_PROCESS_KEY]) {
if (__DEV__) {
error('`setTheme` should not be called during main process.');
}
return;
}

if (this._disposed) {
disposedWarning(this.id);
return;
}
Copy link
Member

Choose a reason for hiding this comment

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

If we provide an API setTheme, I think the behavior should better be similar to setOption. that is, it modifies the model, and causes re-render either immediately or in a lazy way.
But in the current implementation, re-render will not be triggered.

Copy link
Contributor Author

@Ovilia Ovilia Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

By calling updateMethods.prepareAndUpdate, it will re-render as soon as calling setTheme. See the demo of test case:

onclick: function () {
    const riverTheme = {
        backgroundColor: '#ccf',
        color: ['#00f'],
        title: {
            textStyle: {
                color: '#00f'
            }
        }
    };
    chart.setTheme(riverTheme); // renders the chart at once
}

Anything else I need to change?

Copy link
Member

@100pah 100pah Jan 22, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

After reviewing the implementation of ECharts#resize, I updated by understanding.

  • Whether to trigger a event when an API called? The current approach is that a parameter silent is provided, which is supported by all of the ECharts#setOption, ECharts#resize and ECharts#dispatchAction. So in setTheme we'd better follow the convention.
  • Whether to call zr.flush() to perform zr render before the API return?
    • The current impl is not consistent:
      • ECharts#setOption: if not ssr, call zr.flush() immediately
      • ECharts#dispatchAction provide an parameter flush to enable customization it.
      • ECharts#resize does not call zr.flush().
    • If we do not intend to figure this inconsistency this time, I think that it's OK to follow what ECharts#resize does.
  • Whether to use a nested-call-guard: a flag IN_MAIN_PROCESS_KEY is used to prevent from nested calling in both ECharts#setOption, ECharts#resize and ECharts#dispatchAction. So I think we can follow that if we do not want to renew the impl.

So IMO the impl of setTheme can be (modified based on the impl of ECharts#resize):

export interface SetThemeOpts {
    silent?: boolean // by default false.
};


    setTheme(theme: string | ThemeOption, opts?: SetThemeOpts): void {
        if (this[IN_MAIN_PROCESS_KEY]) {
            if (__DEV__) {
                error('`setTheme` should not be called during the main process.');
            }
            return;
        }

        if (this._disposed) {
            disposedWarning(this.id);
            return;
        }

        const ecModel = this._model;
        if (!ecModel) {
            return;
        }

        let silent = opts && opts.silent;
        let updateParams = null as UpdateLifecycleParams;
        // There is some real cases that:
        // chart.setOption(option, { lazyUpdate: true });
        // chart.setTheme('some_theme');
        if (this[PENDING_UPDATE]) {
            if (silent == null) {
                silent = (this[PENDING_UPDATE] as ECharts[PENDING_UPDATE]).silent;
            }
            updateParams = (this[PENDING_UPDATE] as ECharts[PENDING_UPDATE]).updateParams;
            this[PENDING_UPDATE] = null;
        }

        this[IN_MAIN_PROCESS_KEY] = true;

        try {
            this._updateTheme(theme);
            ecModel.setTheme(this._theme);

            prepare(this);
            updateMethods.update.call(this, {type: 'setTheme'}, updateParams);
        }
        catch (e) {
            this[IN_MAIN_PROCESS_KEY] = false;
            throw e;
        }

        this[IN_MAIN_PROCESS_KEY] = false;

        flushPendingActions.call(this, silent);

        triggerUpdatedEvent.call(this, silent);
    }

BTW, it seems that there is some defects in the impl of ECharts#resize: this[PENDING_UPDATE].updateParams is omitted in lazyUpdate scenario. I've no idea whether it does that deliberately or to be a mistake.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks a lot for the help. This deepens my understanding a lot.


const ecModel = this._model;
if (!ecModel) {
return;
}

let silent = opts && opts.silent;
let updateParams = null as UpdateLifecycleParams;

if (this[PENDING_UPDATE]) {
if (silent == null) {
silent = (this[PENDING_UPDATE] as any).silent;
}
updateParams = (this[PENDING_UPDATE] as any).updateParams;
this[PENDING_UPDATE] = null;
}

this[IN_MAIN_PROCESS_KEY] = true;

try {
this._updateTheme(theme);
ecModel.setTheme(this._theme);

prepare(this);
updateMethods.update.call(this, {type: 'setTheme'}, updateParams);
}
catch (e) {
this[IN_MAIN_PROCESS_KEY] = false;
throw e;
}

this[IN_MAIN_PROCESS_KEY] = false;

flushPendingActions.call(this, silent);
triggerUpdatedEvent.call(this, silent);
}

private _updateTheme(theme: string | ThemeOption): void {
if (isString(theme)) {
theme = themeStorage[theme] as object;
}

if (theme) {
theme = clone(theme);
theme && backwardCompat(theme as ECUnitOption, true);
this._theme = theme;
}
}

// We don't want developers to use getModel directly.
Expand Down Expand Up @@ -1660,7 +1715,7 @@ class ECharts extends Eventful<ECEventDefinition> {

prepareAndUpdate(this: ECharts, payload: Payload): void {
prepare(this);
updateMethods.update.call(this, payload, {
updateMethods.update.call(this, payload, payload && {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I kept this change to make the code more robust, although we are not calling this function now.

// Needs to mark option changed if newOption is given.
// It's from MagicType.
// TODO If use a separate flag optionChanged in payload?
Expand Down
5 changes: 5 additions & 0 deletions src/model/Global.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -543,6 +543,11 @@ echarts.use([${seriesImportName}]);`);
return option;
}

setTheme(theme: object) {
this._theme = new Model(theme);
this._resetOption('recreate', null);
}

getTheme(): Model {
return this._theme;
}
Expand Down
100 changes: 87 additions & 13 deletions test/theme.html
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading