| cerealguy | evening, all. interested in contributing. read this page (https://wiki.ubuntu.com/ContributeToUbuntu#Writing_Code). not sure what it means by "Look through the list of Ubuntu specifications on Launchpad. Pick one that interests you, and hopefully there should be enough information to begin with an implementation." can anyone clarify? | 01:02 |
|---|---|---|
| === athairus is now known as afkthairus | ||
| === chihchun_afk is now known as chihchun | ||
| === chihchun is now known as chihchun_afk | ||
| === chihchun_afk is now known as chihchun | ||
| nik90 | davidcalle, ping | 08:02 |
| davidcalle | nik90: pong | 08:13 |
| === sverzegnassi_ is now known as sverzegnassi | ||
| === _salem is now known as salem_ | ||
| t1mp | mardy: hello | 14:06 |
| t1mp | mardy: I was checking out the bug that you reported https://bugs.launchpad.net/ubuntu/+source/ubuntu-ui-toolkit/+bug/1572525 and left a comment | 14:06 |
| ubot5` | Launchpad bug 1572525 in Canonical System Image "[regression] Double header height is set as flickable topMargin" [Undecided,Incomplete] | 14:06 |
| t1mp | oh I see there are more comments added there that I didn't see yet | 14:07 |
| * t1mp reading | 14:07 | |
| mardy | t1mp: well, I don't mind changing my app (if there is a workaround), but I think that such change should be communicated well in advance | 14:09 |
| t1mp | hmm | 14:16 |
| t1mp | indeed, it seems that with the previous version the old topMargin was ignored when setting it to the header height | 14:16 |
| mardy | t1mp: and that's also how it's documented to work | 14:17 |
| t1mp | right | 14:18 |
| t1mp | that needs to be updated | 14:18 |
| t1mp | mardy: would this work for you? https://code.launchpad.net/~tpeeters/ubuntu-ui-toolkit/topMargin-bug1572525/+merge/292386 | 14:26 |
| mardy | t1mp: I don't think that this is the correct approach; I'm writing a comment to the bug, but in short: | 14:27 |
| mardy | t1mp: A) I think that the other bugs can be fixed while still retaining the old behaviour | 14:28 |
| mardy | t1mp: B) I have yet to see a user complaining about the old behaviour (did we have any bugs for that?) | 14:29 |
| mardy | t1mp: C) If you go forawrd with this, please announce it widely, inviting developers to test their apps with the new UITK version | 14:29 |
| t1mp | mardy: A) the change was needed to fix the attached bugs. Storing the old value and reverting it got broken in some cases where multiple PageHeaders are setting the same Flickable (which was happening in some cases even in our UITK gallery app) and it breaks the margins when they set/unset the flickable property in the wrong order (which cannot always be controlled) | 14:34 |
| t1mp | B) I think not, I would have linked those to the MR. But there were concerns and requests to update the behavior for when apps do set a topMargin initially and don't want that reverted by the PageHeader (mainly faenil was arguing for that, I'm not sure if he has an app that needs this) | 14:35 |
| t1mp | C) Right. But I think this landed already. | 14:35 |
| t1mp | I can still send an e-mail to the mailing list | 14:35 |
| mardy | t1mp: it has landed on the rc images; I guess most people are using the stable ones | 14:36 |
| faenil | B) is mainly about common sense. The header was previously deliberately preventing the developers from using a Flickable property | 14:36 |
| faenil | I don't think you need bugs for that, we all agree that shouldn't be how it works :) (I hope :D) | 14:37 |
| mardy | t1mp: let's put it this way: if I find a way to revert the behavioural change while still fixing those bugs, would you consider such a MP? | 14:38 |
| mardy | faenil: no, I don't, it's a property nobody uses (and which nobody could have used, since it was not working right) | 14:38 |
| faenil | mardy: the fact nobody uses it does not justify the old behaviour, imho. It is still a property that should be left available to developers | 14:39 |
| mardy | faenil: I would agree with you if we were starting from a clean plate, but not at this point | 14:39 |
| faenil | mardy: and in fact, you were using it... | 14:39 |
| faenil | mardy: I'd argue the plate is still quite clean, it's better if we fix these things sooner rather than later...but sure :) | 14:39 |
| mardy | faenil: I was using it, because that was the right thing to do given the documented behaviour | 14:40 |
| faenil | mardy: wasn't the documentation saying that Header would set the topMargin? why were you setting it? ;) | 14:40 |
| mardy | faenil: if you want to get it fixed, I would suggest: 1) file a bug and 2) let's fix it by adding an additional property to the header | 14:40 |
| mardy | faenil: please have a look at the attached test case | 14:41 |
| faenil | mardy: I had | 14:41 |
| faenil | I did* | 14:41 |
| mardy | faenil: I was setting the topMargin to the items, so that when they were scrolled into the view, they would already have the proper topMargin (so that the user wouldn't see the page content jump) | 14:42 |
| t1mp | mardy: technically, UITK 1.3 and the PageHeader are still development versions, 1.2 is the stable one. | 14:43 |
| faenil | t1mp: let's start from one thing, isn't it true that mardy's code should have behaved the same with the old code as well? at least by design | 14:44 |
| dpm | t1mp, quick question: what's the standard way of building a qmake-based app? Just run qmake $FILE.pro? Or does the build need to be set up in a different directory, etc? | 14:44 |
| t1mp | mardy: I saw your use case. I think it is not a very common one, but indeed it is one that we did not think of before. | 14:44 |
| faenil | t1mp: i.e. was mardy's code "working" before because of a bug? | 14:45 |
| faenil | (I put working in quotes to mean it behaved as he expected it to behave) | 14:45 |
| t1mp | dpm: I think you don't have to add the file parameter. Just 'qmake' in the correct directory. | 14:46 |
| dpm | t1mp, cool, thanks | 14:46 |
| t1mp | qmake && make, that is | 14:46 |
| t1mp | dpm: if you need out-of-source builds, I think that's possible too. But zbenjamin (and others) would know more about that than I do. | 14:47 |
| faenil | mardy: anyway, I agree it should have been communicated, if it was a behaviour change. Afaik your code should have produced the same behaviour before the change | 14:47 |
| t1mp | faenil: uhm. Theoretically the old code should have behaved like that, but now that I look back at the changes of the MR it looks like the old code did not change the value of topMargin relatively, but just overwrote it with the header height | 14:47 |
| faenil | t1mp: of course it didn't actually behave like that :D but the intention was that it should have, right? | 14:48 |
| faenil | the fact that mardy's code behaviour change was due to a *bug* in the old code | 14:48 |
| faenil | not to the new change, right? | 14:48 |
| t1mp | the old code was supposed to work with flickables that had an initial topMargin of 0 | 14:49 |
| t1mp | so let's say, the behavior for this use case would have been undefined | 14:49 |
| faenil | that's quite a strong assumption :| | 14:49 |
| t1mp | well, if an app has a topMargin in the flickable set, and you use a header with that flickable, in most cases, the intended behavior of the app developer would have broken | 14:51 |
| faenil | t1mp: so, the behaviour of mardy's code, based on your words, was expected to be "undefined" | 14:51 |
| mardy | t1mp: yes, as I understand the old code was *replacing* the topMargin, the new code is *adding* | 14:51 |
| t1mp | so you can call the old behavior a bug, but mardy is right that in this specific case he used that behavior (or bug) to get the desired behavior | 14:51 |
| t1mp | mardy: yes, that is correct | 14:51 |
| faenil | mardy: the old code was ignoring your topMargin, basically | 14:52 |
| faenil | t1mp: of course | 14:52 |
| t1mp | faenil: it ignored until you unset Header.flickable, then it was supposed to restore the old topMargin (which was broken) | 14:52 |
| t1mp | (*which was broken in some corner cases) | 14:52 |
| faenil | yep | 14:52 |
| mardy | faenil: yes, it was overriding it while the flickable was connected to the header; but that's exactly how it was documented to work, so I was not relying on any undefined behaviour | 14:53 |
| faenil | mardy: sorry if I repeat, wasn't the documentation saying that the header would set topMargin? | 14:53 |
| mardy | faenil: correct, it was | 14:54 |
| faenil | and it was also saying that when changing the flickable the topMargin would be reset to 0.. | 14:55 |
| mardy | t1mp: as a sidenote, your workaround works mostly fine; but I lost the back navigation; any way to restore it? | 14:55 |
| faenil | so, I'm not sure I understand why you were expecting your topMargin to have any effect | 14:56 |
| t1mp | mardy: I removed the PageStack from the example code because I thought it was not relevant for the bug | 14:56 |
| t1mp | I tried to simplify it more | 14:56 |
| t1mp | mardy: oh, right. The PageHeader is no longer directly in the Page on top of the PageStack. | 14:56 |
| t1mp | mardy: PageHeader { leadingActionBar.actions: [ Action { text: "back"; iconName: "back"; onTriggered: pageStack.pop() } ] } | 14:57 |
| t1mp | something like that | 14:57 |
| mardy | t1mp: ok, thanks | 14:57 |
| t1mp | faenil: is this a better description? | 14:59 |
| t1mp | * When the header is invisible because its visible property is false, or the header | 14:59 |
| t1mp | * has no parent, its height for the topMargin is considered 0. | 14:59 |
| mardy | faenil: sorry, I missed your last line; so, if I remove the topMargin from my snippet, when I start swiping the screen horizontally, I see the next item appear | 14:59 |
| mardy | faenil: it appears anchored at the top of the screen; then, as soon as it becomes the currentItem, it jumps down (because the topMargin gets changed) | 15:00 |
| mardy | faenil: by setting the topMargin to be the same as the page header, I was avoiding that jump | 15:01 |
| t1mp | mardy: as you can see in the example code I added to the bug, it seems like a case where you would want multiple headers (which was not possible with Ubuntu.Components 1.2) | 15:01 |
| t1mp | mardy: perhaps a designer could comment on what is the best way to think about this | 15:02 |
| mardy | t1mp: maybe; your suggestions works fine, so I'm OK with it -- I'm still not enthusiastic about it, because it makes the delegates heavier (each one has its own header), but I can live with that | 15:03 |
| t1mp | mardy: otherwise, the single header might still jump up/down when you change the flickable, depending on the contentY of the flickable | 15:03 |
| * faenil reads | 15:05 | |
| t1mp | faenil: I had problems coming up with a clear description there | 15:06 |
| t1mp | faenil: what I tried to say is that when you make the header invisible, the header height is subtracted from the topMargin again | 15:06 |
| t1mp | faenil: basically what you requested, the header not to affect the topMargin when it is not visible | 15:06 |
| t1mp | so maybe you have a better description :) | 15:06 |
| faenil | so, just say that it behaves the same way as when you unset the flickable? | 15:06 |
| faenil | (and for the readers: I made that request because that's the way QtQuick layout components work, they ignore items which are !visible or have a null size, not because I'm schizo :D) | 15:07 |
| faenil | (it's to be consistent with what QtQuick developers are expecting) | 15:07 |
| t1mp | yeah, the request makes total sense :) | 15:08 |
| faenil | t1mp: just giving some background, as it seems like I just came and asked for random things, lol | 15:08 |
| t1mp | faenil: right. That's what it usually seems to me like at first, too :) | 15:08 |
| faenil | (and, of course, my requests were discussed with team leads etc :D) | 15:08 |
| faenil | t1mp: LOL | 15:09 |
| * faenil slaps t1mp with a trout | 15:09 | |
| faenil | you deserved that :D :P | 15:09 |
| * t1mp pulls the trout out of faenil's hands and eats it | 15:09 | |
| t1mp | :p | 15:09 |
| faenil | hahhaha | 15:09 |
| faenil | good one | 15:09 |
| t1mp | faenil: * Making the header invisible has the same effect on the topMargin as unsetting | 15:12 |
| t1mp | * the flickable. | 15:12 |
| * zsombi comes to check the chat about the inner-flickable vs PageHeader behavior bug | 15:13 | |
| faenil | mardy: ok, I understand your usecase better now. Let me think (I'm also in quite of a rush with the deadline for a proto :/ ) | 15:16 |
| t1mp | mardy, faenil, zsombi: I propose I update the docs (see the MR attached to the bug), and I send an e-mail to ubuntu-phone | 15:21 |
| zsombi | t1mp: ok, good! | 15:21 |
| t1mp | I guess also to [ubuntu-touch-coreapps] | 15:22 |
| faenil | t1mp: better if we also find a solution to mardy's usecase, as it will be useful. (And maybe we can add that to Header's docs as well) | 15:23 |
| faenil | a solution which is not a hack, that is :) | 15:23 |
| t1mp | faenil: I proposed a solution with a link to a pastebin in my comment on the bug | 15:23 |
| t1mp | I don't think it is a hack | 15:23 |
| zsombi | t1mp: commented | 15:23 |
| zsombi | the MR | 15:23 |
| faenil | t1mp: as mardy says, adding one Header for each flickable is a bit heavy :/ but sure, if we can't find anything else... | 15:24 |
| t1mp | faenil: there are multiple vertical flickables that can be swiped horizontally, and they all use a single header. From UX-point-of-view that seems a bit weird to me. So I think we need UX input for the use case | 15:24 |
| t1mp | Femma: hello :) | 15:24 |
| t1mp | faenil: perhaps you could execute mardy's code and show it to Femma? | 15:25 |
| zsombi | mardy: I still don't get why you need to set the delegate's inner Flickable to control the header... | 15:25 |
| faenil | t1mp: I think that's what you're going to have to implement given the new animations of the Sections component, instead :P | 15:25 |
| faenil | (you remember? :D) | 15:25 |
| t1mp | zsombi: ok. I pushed an update. | 15:26 |
| t1mp | faenil: new animations??? | 15:27 |
| t1mp | faenil: ah, you mean the swiping between pages | 15:27 |
| faenil | t1mp: sdk/design sync, a couple of weeks ago... | 15:27 |
| faenil | yes | 15:27 |
| faenil | between Sections .p | 15:27 |
| t1mp | faenil: I think of Sections as a separate component. The swiping would be between Pages :) | 15:27 |
| t1mp | but they would need to be linked to Sections | 15:28 |
| faenil | t1mp: doesn't matter :) it's still 1 header and multiple pages | 15:28 |
| t1mp | faenil: hmm, yeah something to think about, because also each Page has its own header now.. | 15:28 |
| t1mp | I don't remember if transitions in the header were included in the designs | 15:29 |
| faenil | I think there was no transition in the header, because I remember raising the problem "we're in trouble" :D | 15:30 |
| faenil | mardy: I thought topMargin: parent.ListView.isCurrentItem ? 0 : mainPage.header.height | 15:34 |
| faenil | would help | 15:34 |
| faenil | and it does, at the beginning | 15:35 |
| faenil | but then Header breaks that binding with its assignment | 15:35 |
| faenil | so it only works at the beginning.. | 15:35 |
| === chihchun is now known as chihchun_afk | ||
| t1mp | mardy: actually your code seems to work fine for me... | 15:39 |
| t1mp | I don't know why | 15:39 |
| zsombi | t1mp: you hold it right then :D | 15:39 |
| t1mp | nevermind, I know why. Indeed with an older UITK it behaved differently | 15:39 |
| faenil | lol :D | 15:43 |
| t1mp | I'm wondering whether maybe the header should just be locked in this case | 15:47 |
| faenil | mardy: do you see any way how to make everyone happy without changing the API? | 15:50 |
| t1mp | mardy: does this code in the Flickable do what you want? topMargin: ListView.isCurrentItem ? 0 : mainPage.header.height | 15:50 |
| t1mp | I think you'll get some unwanted topMarginChanged events there while the values are updating.... | 15:51 |
| faenil | mardy: it doesn't work after Header breaks the bindinwg | 15:51 |
| faenil | it only works at the beginning | 15:51 |
| t1mp | oh right it does break the binding | 15:54 |
| zsombi | t1mp: happrove | 16:05 |
| zsombi | +d | 16:05 |
| t1mp | zsombi: thanks | 16:06 |
| t1mp | I guess we'll still have some discussion about mardy's use case, but the doc fix is valid anyway | 16:06 |
| t1mp | I sent the e-mail on the mailing lists | 16:30 |
| === shuduo is now known as shuduo-afk | ||
| === chihchun_afk is now known as chihchun | ||
| bzoltan_ | mhall119: do you know how to convince the webapp container to start with a given zoomed mode? | 17:05 |
| bzoltan_ | dbarth: alex_abreu: ^ | 17:09 |
| alex_abreu | bzoltan_, it is not possible atm | 17:10 |
| alex_abreu | bzoltan_, you should add a feature request | 17:10 |
| alex_abreu | bzoltan_, but I am not sure how useful it would be | 17:11 |
| bzoltan_ | alex_abreu: there are silly webservices where the layout is odd...ie. for example it scales well to landscape, but does not adapt to portrait mode ... a forced double zoom would help. But it is not that important | 17:12 |
| alex_abreu | bzoltan_, ok, I am not sure it would qualify as proper general use cases yet :) | 17:14 |
| bzoltan_ | alex_abreu: I appreciate your very gentle and kind way to say that "bzoltan_ that is stupid, make up some better usecase" :D | 17:17 |
| alex_abreu | bzoltan_, well not really that ... but it would need a stronger use case indeed :) | 17:20 |
| === chihchun is now known as chihchun_afk | ||
| === dpm is now known as dpm-mostly-afk | ||
| t1mp | mardy: did you say that the change in the PageHeader affected your app when installing rc-proposed? it looks like the last UITK version that we released is r1938 which does not include the change | 20:19 |
| t1mp | ah no, not true. I was looking at the wrong change. This was r1931 so it is released | 20:20 |
| === afkthairus is now known as athairus | ||
| === salem_ is now known as _salem | ||
Generated by irclog2html.py 2.7 by Marius Gedminas - find it at mg.pov.lt!