Bug 88410 – [Qt] Improve the visual of the viewport info box

• Status: RESOLVED
• Resolution: FIXED
• Component: WebKit Misc.

[Kenneth Rohde Christiansen]

SSIA

[Kenneth Rohde Christiansen]

Created attachment 146005 [details]
Patch

[Kenneth Rohde Christiansen]

Created attachment 146006 [details]
Patch

[Michael Brüning]

Comment on attachment 146006 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146006&action=review

LGTM (two comments below) :) - Maybe Tor Arne can have a look too and r+?

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:25
> +        anchors.margins: 10

As discussed with Tor Arne on irc, anchors { ... } would be the preferred way.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:37
> +

Same as above regarding the anchors.

[Kenneth Rohde Christiansen]

Created attachment 146007 [details]
Patch

[Kenneth Rohde Christiansen]

Created attachment 146010 [details]
Patch

[Michael Brüning]

Comment on attachment 146010 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=146010&action=review

Looks good in general. but there are a couple of problems with the property access, which leads to accessing undefined properties and hence warnings.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:78
> +                        font.family: parent.fontFamily

This does not seem to work properly as the font.family is defined in the parent's grandparent (i.e. the Text's parent is the inner column, the inner Column's parent is the outer Column, the outer columns parent is the Item which has the property) - MiniBrowser put's out some warnings about undefined properties on the console.

You could either go up the hierarchy (parent.parent.parent) or assign an id to the Item (e.g. "infoColumContainer"?) to access the properties.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:79
> +                        color: parent.fontColor

The above applies to fontColor as well.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:86
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:87
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:91
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:92
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:96
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:97
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:104
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:105
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:109
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:110
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:114
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:115
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:122
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:123
> +                        color: parent.fontColor

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:127
> +                        font.family: parent.fontFamily

Same as above.

> Tools/MiniBrowser/qt/qml/ViewportInfoItem.qml:128
> +                        color: parent.fontColor

Same as above.

[Kenneth Rohde Christiansen]

Created attachment 146018 [details]
Patch

[Michael Brüning]

Looks good to me.

[Kenneth Rohde Christiansen]

Landed in r119710