rschroll / beru

The Basic Epub Reader for Ubuntu
http://rschroll.github.io/beru/
GNU General Public License v3.0
24 stars 12 forks source link

Binding loop for detected for property "height" #17

Closed stuartlangridge closed 10 years ago

stuartlangridge commented 11 years ago

When I scroll the list-of-books listview, I get a zillion errors of QML Page: Binding loop detected for property "height". I think the LocalBooks code is using Page a bit wrong, but I don't know how to use it right. I boiled this down to a tiny testcase and have posted to G+ about it in the hope that someone with more QML expertise will explain how to use Page better... https://plus.google.com/u/0/108243663090085262773/posts/4oeVpiuN2RT

stuartlangridge commented 11 years ago

Got a useful answer from @mardy. It seems that this is really a bug in Page (well, debateably so). Anyway, there is a fix at https://github.com/stuartlangridge/beru/commit/80e436dc9ba8ad6076d66ae411ec74155852f9fc but I'm not at all sure that that's OK; I don't really understand the two-column wide stuff, so I think this might have broken it...

rschroll commented 11 years ago

I haven't tested it, but I suspect it will mess up the two-column behavior, since that relies on positioning rather than anchoring to get everything in the right place. I wonder if the anchoring is needed, though. Is it enough just to change the definition of the hight from the page height to the mainview height?

I've been seeing a similar error, but I'm not sure it's the same one you're seeing. It appears when scrolling through the LocalBooks listview, but it refers to the BookPage height, which is rather odd.

stuartlangridge commented 11 years ago

I don't know about the height thing. It might be enough, indeed: I think the binding problem is that the GridView (note that my branch is using a gridview so that I can do the cover browser! so don't just merge it wholesale :)) defines its height in terms of its parent (LocalBooks) and LocalBooks defines its height in terms of the mainView, but LocalBooks sets its flickable to be the GridView and that automatically tries to set the parent height, and so they depend on one another. I think. QML is wildly unhelpful at debugging this sort of error. Anyway, this may help, at least a little :)

On Fri, Sep 13, 2013 at 4:06 PM, rschroll notifications@github.com wrote:

I haven't tested it, but I suspect it will mess up the two-column behavior, since that relies on positioning rather than anchoring to get everything in the right place. I wonder if the anchoring is needed, though. Is it enough just to change the definition of the hight from the page height to the mainview height?

I've been seeing a similar error, but I'm not sure it's the same one you're seeing. It appears when scrolling through the LocalBooks listview, but it refers to the BookPage height, which is rather odd.

— Reply to this email directly or view it on GitHubhttps://github.com/rschroll/beru/issues/17#issuecomment-24400989 .

New Year's Day -- everything is in blossom! I feel about average. -- Kobayashi Issa

rschroll commented 11 years ago

This patch gets rid of the error I was seeing. I hope it kills yours as well; if not please reopen.

rschroll commented 11 years ago

I had to revert this commit because it was making the BookPage toolbar appear on the LocalBooks page when the app was first run. I have no idea how these are related, but they are.

rschroll commented 10 years ago

I'm not seeing these problems anymore. Hopefully they're gone forever, but if they reappear, please reopen.