[Merge] lp:~artmello/webbrowser-app/webbrowser-app-history_view_convergence into lp:webbrowser-app

Olivier Tilloy olivier.tilloy at canonical.com
Tue Aug 4 10:48:18 UTC 2015


> I added a couple of new tests but they are in a no good state so far.
> I am facing problem in populating the history model before running the
> tests. It seems that the tests are running before everything is
> correctly set. So unfortunately that is still WIP.

You shouldn’t use a timer to populate the model asynchronously. You want to ensure that your model is fully populated in init(). Use the wait() method for that purpose: http://doc.qt.io/qt-5/qml-qttest-testcase.html#wait-method.


> I am using that since something is setting the focus after the
> component is complete. It needs further investigation to see what is
> happening and how to unset that.

Indeed it needs investigation. This workaround cannot go as-is in production without at the very least some understanding of what’s happening. Most likely though, once understood the workaround will become unnecessary.


> Both those questions are related with how I implemented the left panel.

Right. My suggestion with empty delegates won’t work wrt keyboard navigation, so forget about it.
So we need a custom model indeed. Given that, can’t we insert the "All history" entry at index 0 in that model, and get keyboard navigation for free, without having to misuse the header? The 'lastVisitDate' role would return an invalid date for "All history", and the view would know how to deal with it.

As I pointed out in my previous comment, I don’t think the "entries" role is necessary for that model, so the m_lastVisitDates member could be removed.
-- 
https://code.launchpad.net/~artmello/webbrowser-app/webbrowser-app-history_view_convergence/+merge/263052
Your team Ubuntu Phablet Team is subscribed to branch lp:webbrowser-app.



More information about the Ubuntu-reviews mailing list