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.

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

Resize method for ui.echart #1570

Merged
merged 6 commits into from
Sep 18, 2023
Merged

Resize method for ui.echart #1570

merged 6 commits into from
Sep 18, 2023

Conversation

dabenny
Copy link
Contributor

@dabenny dabenny commented Sep 6, 2023

Add a method to resize the chart after the creation

Add a method to resize the chart after the creation
@rodja
Copy link
Member

rodja commented Sep 7, 2023

Thanks for the pull request. We have the issue #1563 which could be solved/simplified with this code. But I'm not entirely sure it's all we need. See #1563 (comment).

@natankeddem
Copy link
Contributor

I propose the following additions. This will extend the resize to a element method and will also call it automatically when the container changes size.

echarts.js

export default {
  template: "<div></div>",
  mounted() {
    this.chart = echarts.init(this.$el);
    this.chart.setOption(this.options);
    new ResizeObserver(() => this.resize_chart()).observe(this.$el);
  },
  beforeDestroy() {
    this.destroyChart();
  },
  beforeUnmount() {
    this.destroyChart();
  },
  methods: {
    update_chart() {
      if (this.chart) {
        this.chart.setOption(this.options);
      }
    },
    resize_chart() {
      if (this.chart) {
        this.chart.resize();
      }
    },
    destroyChart() {
      if (this.chart) {
        this.chart.dispose();
      }
    },
  },
  props: {
    options: Object,
  },
};

echarts.py

from typing import Dict

from ..element import Element


class EChart(Element, component='echart.js', libraries=['lib/echarts/echarts.min.js']):

    def __init__(self, options: Dict) -> None:
        """Apache EChart

        An element to create a chart using `ECharts <https://echarts.apache.org/>`_.
        Updates can be pushed to the chart by changing the `options` property.
        After data has changed, call the `update` method to refresh the chart.

        :param options: dictionary of EChart options
        """
        super().__init__()
        self._props['options'] = options
        self._classes = ['nicegui-echart']

    @property
    def options(self) -> Dict:
        return self._props['options']

    def update(self) -> None:
        super().update()
        self.run_method('update_chart')

    def resize(self) -> None:
        self.run_method('resize_chart')

@dabenny
Copy link
Contributor Author

dabenny commented Sep 8, 2023

Hi, thanks for looking on this PR.
My PR is not related to the issue #1563, I just need to resize the chart after its creation.
The echart canvas dimensions does not change after an update but only after a resize so, I just suggest the addition of the method.

The addition by @natankeddem fully solve automatically my needs.

@falkoschindler falkoschindler linked an issue Sep 15, 2023 that may be closed by this pull request
@falkoschindler falkoschindler changed the title Update echart.js Resize method for ui.echart Sep 15, 2023
@natankeddem
Copy link
Contributor

natankeddem commented Sep 15, 2023

Hi, thanks for looking on this PR. My PR is not related to the issue #1563, I just need to resize the chart after its creation. The echart canvas dimensions does not change after an update but only after a resize so, I just suggest the addition of the method.

The addition by @natankeddem fully solve automatically my needs.

If these additions worked, you might want to update the PR so the maintainers can review and hopefully merge this fix in. Thank you for trying my proposal out.

@dabenny
Copy link
Contributor Author

dabenny commented Sep 18, 2023

Hi, thanks for looking on this PR. My PR is not related to the issue #1563, I just need to resize the chart after its creation. The echart canvas dimensions does not change after an update but only after a resize so, I just suggest the addition of the method.
The addition by @natankeddem fully solve automatically my needs.

If these additions worked, you might want to update the PR so the maintainers can review and hopefully merge this fix in. Thank you for trying my proposal out.

I'm sorry, I'm not so experienced.
I updated the forked branch with your suggestions. Is this enough to update also the pull request?
Thanks for your advice

@falkoschindler falkoschindler added this to the 1.3.14 milestone Sep 18, 2023
Copy link
Contributor

@falkoschindler falkoschindler left a comment

Choose a reason for hiding this comment

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

Thanks for this PR, @dabenny! Looks good to me!
This is the code I used to try it out:

with ui.expansion('Expansion').classes('w-full'):
    echart = ui.echart({
        'xAxis': {'type': 'value'},
        'yAxis': {'type': 'category', 'data': ['A', 'B']},
        'series': [{'type': 'bar', 'name': 'Alpha', 'data': [0.1, 0.2]}],
    })

I only added a short explanation about refresh. But usually you won't need this method thanks to the ResizeObserver.

I updated the forked branch with your suggestions. Is this enough to update also the pull request?

Yes, it is. 👍🏻


Update: I figured we shouldn't introduce a resize() method if it isn't needed. So to avoid potential confusion, I removed it altogether. And I found some conditions in echart.js that can be removed (in contrast to chart.js) while we're here.

@falkoschindler falkoschindler merged commit 035f059 into zauberzeug:main Sep 18, 2023
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

EChart width on initial load
4 participants