webbrowser-app

Code review comment for lp://qastaging/~gerboland/webbrowser-app/formFactor-support

formFactor-support
Merge into trunk

Revision history for this message

Chris Coulson (chrisccoulson) wrote on 2016-02-22:

> > I’m not seeing any formFactor property defined on UserAgent02, so I doubt
> the
> > onFormFactorChanged handler in UbuntuWebContext.qml would work.
>
> Code untested. Was struggling to cross compile & deploy, sbuild unhappy with
> me today.
>
> > Note that I’m currently working on a branch that removes the 'formFactor'
> > property from the Ubuntu.Web plugin’s context object¹, so it’s kind of
> > ironical that you’re adding it back in another branch. I really don’t think
> > the default user agent (and corresponding overrides) should be tied to the
> > "form factor" (whatever that means in a world of convergence). How does mir
> > determine whether a device is a phone or a desktop, or a tablet? Is that a
> > solid API that we are going to support and publicize?
>
> I obviously have no idea of your plans, this was just an attempt at replacing
> your screen heuristic with something more concrete.
>
> Form Factor will be a hint to the application to tell it what mode the shell
> is in, on each screen. Mir supports it, shell will do its best to set it
> correctly. I would hope the average app author has no need for such a
> property, any form factor specific behaviour would be encapsulated in the
> UITK.
>

It sounds like this might be useful for bug 1545088, bug 1547145 and bug 1547102. How does the shell determine what the hint is?

> > Even if that was a sensible thing to do, we most probably don’t want to get
> a
> > mobile UX on a tablet. Well, at least not a 10" tablet. On a 7" tablet
> maybe.
> > See where the screenDiagonal property comes from?
>
> Is your call. The goal of this was to indicate that you can learn if the app
> is running on a tablet or a desktop UI at the time.
>
> > As a side note, the dependency on Qt5Gui_PRIVATE is not very welcome, given
> > that we’ve been steadily working towards entirely removing the dependencies
> on
> > private Qt packages (ask Mirv about it).
>
> Qt doesn't have a nice way to export it to apps yet, hence having to resort to
> private Qt headers. As I mentioned in a comment, a better solution is for the
> UITK to do that for you. This isn't the final solution.
> Thanks for the comments!

> > I’m not seeing any formFactor property defined on UserAgent02, so I doubt
> the
> > onFormFactorChanged handler in UbuntuWebContext.qml would work.
> 
> Code untested. Was struggling to cross compile & deploy, sbuild unhappy with
> me today.
> 
> > Note that I’m currently working on a branch that removes the 'formFactor'
> > property from the Ubuntu.Web plugin’s context object¹, so it’s kind of
> > ironical that you’re adding it back in another branch. I really don’t think
> > the default user agent (and corresponding overrides) should be tied to the
> > "form factor" (whatever that means in a world of convergence). How does mir
> > determine whether a device is a phone or a desktop, or a tablet? Is that a
> > solid API that we are going to support and publicize?
> 
> I obviously have no idea of your plans, this was just an attempt at replacing
> your screen heuristic with something more concrete.
> 
> Form Factor will be a hint to the application to tell it what mode the shell
> is in, on each screen. Mir supports it, shell will do its best to set it
> correctly. I would hope the average app author has no need for such a
> property, any form factor specific behaviour would be encapsulated in the
> UITK.
>

It sounds like this might be useful for bug 1545088, bug 1547145 and bug 1547102. How does the shell determine what the hint is?

> > Even if that was a sensible thing to do, we most probably don’t want to get
> a
> > mobile UX on a tablet. Well, at least not a 10" tablet. On a 7" tablet
> maybe.
> > See where the screenDiagonal property comes from?
> 
> Is your call. The goal of this was to indicate that you can learn if the app
> is running on a tablet or a desktop UI at the time.
> 
> > As a side note, the dependency on Qt5Gui_PRIVATE is not very welcome, given
> > that we’ve been steadily working towards entirely removing the dependencies
> on
> > private Qt packages (ask Mirv about it).
> 
> Qt doesn't have a nice way to export it to apps yet, hence having to resort to
> private Qt headers. As I mentioned in a comment, a better solution is for the
> UITK to do that for you. This isn't the final solution.
> Thanks for the comments!

« Back to merge proposal