Code review comment for lp:~ubuntu-sdk-team/ubuntu-ui-toolkit/visualRoot

> > The property holds the window's visual root,
> > as opposed to the root item.
> Should we prepend this sentence with 'If set,'?
> To make it clear that visualRoot is ignored
> when it is null? Or say 'set to null in order
> to use the root item....'.

I moved the If to the start of the sentence.

> > If set, popups (popovers, dialogs, menus) will
> > reparent to it when opened via
> They won't be reparented, they will be created
> as children of visualRoot (as you say in the
> docs of popupUtils).

Yes and no. popupUtils.open creates them initially with the parent. show() on the other hand does reparent after the fact if reparentToRootItem is true.

I made this more explicit now.

> > - QQuickItem *testItem(QQuickItem *that, const QString &identifier)
> > + QQuickItem *testItem(QObject *that, const QString &identifier)
> why does this have to be changed?

A QQuickWindow is not a QQuickItem. Properties and children are however implemented in QObject and aren't specific to items at all. So we can use them with a window just fine, so long as we don't try to cast it to something unrelated that it isn't.