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

Handle resize with ResizeObserver if it's exist in window #71

Closed
kritsky opened this issue Jun 17, 2019 · 15 comments · Fixed by #599
Closed

Handle resize with ResizeObserver if it's exist in window #71

kritsky opened this issue Jun 17, 2019 · 15 comments · Fixed by #599
Assignees
Labels
enhancement Feature requests, and general improvements.
Milestone

Comments

@kritsky
Copy link

kritsky commented Jun 17, 2019

Lightweight Charts Version:
Latest

Steps/code to reproduce:
Just rotate the device

Actual behavior:
Fixed width always

Expected behavior:
Need to be responsive

Screenshots
Here also: https://www.tradingview.com/lightweight-charts/

@timocov
Copy link
Contributor

timocov commented Jun 17, 2019

It looks like there is no way to handle resize of some DOM element (except ResizeObserver, but it is supported in Chrome only at the moment). I believe that in this case you need to handle resize event of window and then call resize method (or applyOptions with the new width and height options) on every resize if you decide that size of the element is changed.

@kritsky
Copy link
Author

kritsky commented Jun 17, 2019

I'm using this one: var container = document.getElementById('chart');
And css is: #chart{height:20rem;width:100%}

@kritsky
Copy link
Author

kritsky commented Jun 17, 2019

Check this out: https://www.chartjs.org/samples/latest/charts/line/basic.html
And try to resize.

@timocov
Copy link
Contributor

timocov commented Jun 17, 2019

Check this out

Ah, see. They measure node's size on every resize of the window and apply this size on the chart. But if only CSS is changed then they don't resize the chart and the problem is still here (just open the devtools and change a width of the chart's container - nothing is changed, but if you resize the window then, it'll adopt to the current state - how do you handle this in your code?).

I believe that the best way to handle container's width/height is your code because only you know how your webpage works, when you resize elements and so on. But why you should pay for handling window resize, if you're sure that nothing changed in the current event? We trying to do nothing which can affects the webpage performance.

I believe we can try to add fallback to ResizeObserver if it's available in the browser to resize the chart, but I'm not sure that handling resize of the window is good.

/cc @subzey @ezhukovskiy @eugene-korobko

@kritsky
Copy link
Author

kritsky commented Jun 17, 2019

It would be nice. Your TV app do the same. It resizes when I change layout.

@timocov
Copy link
Contributor

timocov commented Jun 17, 2019

Your TV app do the same. It resizes when I change layout.

The TV app is the client of the chart as you are - it request a resize when it decided that chart's size is changed, but the chart does nothing to determine it.

It would be nice

But it will work in Chrome only for now...

@timocov
Copy link
Contributor

timocov commented Jul 11, 2019

Hey @kritsky, ResizeObserver has very poor support worldwide and I don't think that it's time for it.

@kritsky
Copy link
Author

kritsky commented Jul 11, 2019

Is it good user experience?
Vertical layout on iPhone: https://yadi.sk/i/hivXTbt7R12Wew
And switched to horizontal layout: https://yadi.sk/i/xxLWrwHQrnAc5g

@timocov
Copy link
Contributor

timocov commented Jul 11, 2019

Is it good user experience?

As I said before: without ResizeObserver we can't get a notification about resizing of the container.

Why we should forcing resize the chart if it's size isn't changed (for example, if charts on your page have the same width/height in any layout)?
What if you page has several layout breakpoints (for instance 320px, 640px, 1100px) and between them nothing is changed on the layout: we'll get a lot of resize events and we'll generating force reflow by requesting a size of the container, because we don't know whether is something changed or not. What's about this case? Force reflow is slow and may reduce the performance of the whole page. We don't want it.

Window's resize event doesn't actually mean that chart's size is changed, and it's size can be changed just by adding a new CSS class anywhere on the page (or even without it - e.g. if you use CSS pseudo-classes). How do you handle it? We can't know it and there is no window's resize event when it's happened.

The only you, as the website developer, know what's going on on your website, what layout you have, how and when you need to handle changes.

@timocov timocov added the question General question. label Jul 22, 2019
@timocov timocov mentioned this issue Sep 3, 2019
@odlainepre
Copy link

@kritsky

.chart {
position: absolute;
width: 100%;
Height: 100%
}
// https://www.npmjs.com/package/resize-sensor

new ResizeSensor(document.querySelector('.chart'), event => {
  chart.resize(document.querySelector('.chart').offsetHeight, document.querySelector('.chart').offsetWidth)
})

@timocov
Copy link
Contributor

timocov commented Sep 5, 2019

Or even d02a1d3#diff-f84d64383e454ee3a64791a19691f324R105-R115, but IMO currently it should be done outside of the lightweight-charts.

@timocov
Copy link
Contributor

timocov commented Jun 1, 2020

Time is going and it looks like all major browsers support ResizeObserver API (Safari 13.1 was released some months ago) so we can consider to add new option like handleResize: boolean with default value 'ResizeObserver' in window and handle resize like I shown in d02a1d3#diff-f84d64383e454ee3a64791a19691f324R105-R115 (if no ResizeObserver exists in window we won't use it or require for polyfill even if handleResize is enabled). Thoughts?

@timocov timocov added enhancement Feature requests, and general improvements. need more feedback Requires more feedback. and removed question General question. labels Jun 1, 2020
@timocov timocov changed the title When rotate smartphone it doesn't change width Handle resize with ResizeObserver if it's exist in window Jun 1, 2020
@timocov timocov added this to the Future milestone Jun 1, 2020
@timocov timocov removed the need more feedback Requires more feedback. label Jun 1, 2020
@Guillerming
Copy link

Guillerming commented Jun 12, 2020

For those who're looking a way to make any chart fully responsive, the trick I've used is using a parent element as a size-guide, hopefully it will be helpful for some of you.

We start setting up a wrapper to our chart-element. I'm naming it .parent-element

<div class="parent-element">
   <div class="chart-element"></div>
</div>

The CSS may look like as follows.

.parent-element {
  position: relative;
  width: 100%;
  padding-bottom: 50%;
}
.child-element {
  position: absolute;
  top: 0;
  left: 0;
  width: 100%;
  height: 100%;
}

Now that .parent-element behaves as a solid container (of an aspect ratio of 2:1) increasing/decreasing height according to its width which is set to fit the 100% of the available X space, all we need to do is to attach the window resizing event to our chart applyOptions method passing new width and height set by reading .parent-element's width and height respectively.
Using jQuery now but you can do that with plain js if you like:

$(window).resize(function() {
   myChartHandler.applyOptions({
    width: $('.parent-element').outerWidth(),
    height: $('.parent-element').outerHeight()
  });
});

Your chart is by now fully responsive and listens to window resizes to adapt. But if for any reason you need to force resizing for a change on your layout that happens for pressing a button or for anything other than changing window's size, just trigger the resize event whenever you need:

$(window).trigger('resize');

It may also be helpful to fit your chart within the current proportions by executing fitContent() method, but this depends entirely on your requirements and wishes.

myChartHandler.timeScale().fitContent();

@timocov timocov modified the milestones: Future, 3.2 Jun 29, 2020
@SuperPenguin
Copy link
Contributor

I just stumbled upon this issue, and want to point out that ResizeObserver spec actually changed early this year. @timocov might want to update ResizeObserver definitions from here. It should cover most chrome, firefox, and safari that support ResizeObserver natively (new and old spec). I could make PR to update definitions and implementations in resize-observer branch if you want.

@timocov
Copy link
Contributor

timocov commented Oct 8, 2020

@SuperPenguin thanks for the update! It's possible that this issue might be fixed in #367 (or at least could be depend on), so let's wait it first. If there will be something to do, I can ping you if you wish.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Feature requests, and general improvements.
Projects
None yet
7 participants