search

phibr0 / obsidian-charts

Charts - Obsidian Plugin | Create editable, interactive and animated Charts in Obsidian via Chart.js
https://charts.phib.ro/
GNU Affero General Public License v3.0
592 stars 27 forks source link

[Bug]: Height set to zero when data changed and switched to reading mode #51

Closed kometenstaub closed 2 years ago

kometenstaub commented 2 years ago

Describe the bug

image

Relevant errors (if available)

Uncaught TypeError: Cannot read properties of null (reading 'clientHeight')
    at Sh.onunload (eval at <anonymous> (app.js:1:1), <anonymous>:54:5625)
    at Sh.reload (eval at <anonymous> (app.js:1:1), <anonymous>:54:5399)
    at Sh.changeHandler (eval at <anonymous> (app.js:1:1), <anonymous>:54:5376)
    at e.t.tryTrigger (app.js:1:880519)
    at e.t.trigger (app.js:1:880452)
    at e.trigger (app.js:1:1106848)
    at e.<anonymous> (app.js:1:1104472)
    at app.js:1:234991
    at Object.next (app.js:1:235096)
    at s (app.js:1:233835)
onunload @ VM149:54
reload @ VM149:54
changeHandler @ VM149:54
t.tryTrigger @ app.js:1
t.trigger @ app.js:1
e.trigger @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
s @ app.js:1
setTimeout (async)
t.tryTrigger @ app.js:1
t.trigger @ app.js:1
e.trigger @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
(anonymous) @ app.js:1
Promise.then (async)
t.queue @ app.js:1
e.onCreateOrModify @ app.js:1
t.tryTrigger @ app.js:1
t.trigger @ app.js:1
e.trigger @ app.js:1
e.onChange @ app.js:1
t.trigger @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
t.reconcileFileCreation @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
t.reconcileFileInternal @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
t.reconcileFile @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
t.reconcileInternalFile @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
(anonymous) @ app.js:1
n @ app.js:1
Promise.then (async)
t.queue @ app.js:1
t.write @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
e.modify @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
(anonymous) @ app.js:1
s @ app.js:1
Promise.then (async)
l @ app.js:1
(anonymous) @ app.js:1
A @ app.js:1
e.save @ app.js:1
a @ app.js:1
setTimeout (async)
l @ app.js:1
n.requestSave @ app.js:1
(anonymous) @ app.js:1
a @ app.js:1
setTimeout (async)
l @ app.js:1
(anonymous) @ app.js:1
t.update @ app.js:1
_dispatch @ app.js:1
t.dispatch @ app.js:1
Jx @ app.js:1
eS @ app.js:1
nS @ app.js:1
c @ app.js:1
rc @ app.js:1
keydown @ app.js:1
t.runCustomHandlers @ app.js:1
(anonymous) @ app.js:1
VM149:54 Uncaught TypeError: Cannot read properties of null (reading 'clientHeight')
    at Sh.onunload (eval at <anonymous> (app.js:1:1), <anonymous>:54:5625)
    at Sh.t.unload (app.js:1:673671)
    at e.t.removeChild (app.js:1:673910)
    at t.cleanupParentComponents (app.js:1:866306)
    at t.onRender (app.js:1:856220)

Steps to reproduce

  1. Make a table and link a chart to it, enable backlinks in document (it doesn't need a backlink, just the panel at the end of the note)
  2. Change an amount in source or live preview.
  3. Switch to reading mode.
  4. You'll see what the screenshot shows.

Expected Behavior

It doesn't set the height to zero after changing a value and switching to reading mode.

Additional context

HTML and CSS on initial load:

image

HTML and css after a change in the data and switching to reading mode:

image

Code

|             Day                    |             Amount             |             When                            |        points      |          Condition                                                                                                                     |
|:-----------------------------------|:-------------------------------|:--------------------------------------------|:-------------------|:---------------------------------------------------------------------------------------------------------------------------------------|
|                      2022-03-24    |                  1             |             Evening                         |          10        | n/a                                                                                                                                    |
|                      2022-03-25    |                             5  |             Morning and evening             |           9        |                n/a                                                                                                                     |
|                      2022-03-27    |                             8  |             Morning and evening             |           5        |          n/a                                                                                                                           |
|                     2022-03-28     |                            5   |     Morning                                 |              4     |     n/a                                                                                                                                |

^mytable

```chart
type: line
id: mytable
select: [Amount, points]
tension: 0.2
labelColors: false
fill: false
beginAtZero: true


### Operating system

Linux
lishid commented 2 years ago

Technical analysis:

https://github.com/phibr0/obsidian-charts/blob/10a294f252e8f0eb7173d165f201969a725c57cb/src/chartRenderer.ts#L300 This is likely unnecessary because you're unloading anyway, but what happens is that this might have gotten triggered at a bad time and sets the height to 0.

Even for a reload, this shouldn't really be needed since the height was never set in the first place so it would have just fit the contents inside automatically.

phibr0 commented 2 years ago

I added that because another User reported scrolling Issues in #38 (There are more problems in that Issue that I have to fix). I haven't looked too much into how the RenderChild actually works, but I thought maybe the scrolling issues stop if the height doesn't change when the chart gets destroyed.

Will remove that line and investigate further.

lishid commented 2 years ago

Hmm what you may be dealing with is the fact that cm6 optimizes the rendering by taking out elements that are not in view. clientHeight will be 0 when an element is detached from DOM, so at the very least you can test for that.

There's one other thing you could consider using and it's the Element.onNodeInserted(callback) helper function that Obsidian provides - you'll get a call every time the element is inserted and displayed in the app. Attach this on your canvas and from there you can possibly do some checks and if it's in a screwed up state, you can consider re-rendering.