Bug 88410 - [Qt] Improve the visual of the viewport info box
Summary: [Qt] Improve the visual of the viewport info box
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kenneth Rohde Christiansen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2012-06-06 06:10 PDT by Kenneth Rohde Christiansen
Modified: 2012-06-07 05:12 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.02 KB, patch)
2012-06-06 06:11 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (8.00 KB, patch)
2012-06-06 06:13 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (7.70 KB, patch)
2012-06-06 06:27 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (7.78 KB, patch)
2012-06-06 06:38 PDT, Kenneth Rohde Christiansen
no flags Details | Formatted Diff | Diff
Patch (7.82 KB, patch)
2012-06-06 07:18 PDT, Kenneth Rohde Christiansen
vestbo: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kenneth Rohde Christiansen 2012-06-06 06:10:44 PDT
SSIA
Comment 1 Kenneth Rohde Christiansen 2012-06-06 06:11:28 PDT
Created attachment 146005 [details]
Patch
Comment 2 Kenneth Rohde Christiansen 2012-06-06 06:13:23 PDT
Created attachment 146006 [details]
Patch
Comment 3 Michael Brüning 2012-06-06 06:18:32 PDT
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.
Comment 4 Kenneth Rohde Christiansen 2012-06-06 06:27:24 PDT
Created attachment 146007 [details]
Patch
Comment 5 Kenneth Rohde Christiansen 2012-06-06 06:38:58 PDT
Created attachment 146010 [details]
Patch
Comment 6 Michael Brüning 2012-06-06 07:08:38 PDT
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.
Comment 7 Kenneth Rohde Christiansen 2012-06-06 07:18:18 PDT
Created attachment 146018 [details]
Patch
Comment 8 Michael Brüning 2012-06-06 07:22:13 PDT
Looks good to me.
Comment 9 Kenneth Rohde Christiansen 2012-06-07 05:12:04 PDT
Landed in r119710