| Summary: | [Qt] Add UI for JavaScript Alert dialog in the Qt MiniBrowser | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Alexander Færøy <ahf> | ||||||
| Component: | WebKit Qt | Assignee: | Alexander Færøy <ahf> | ||||||
| Status: | RESOLVED FIXED | ||||||||
| Severity: | Normal | CC: | michael.bruning, vestbo, webkit.review.bot | ||||||
| Priority: | P2 | ||||||||
| Version: | 528+ (Nightly build) | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Attachments: |
|
||||||||
|
Description
Alexander Færøy
2012-01-03 06:53:05 PST
Created attachment 120941 [details]
Patch
Adding Tor Arne. Comment on attachment 120941 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=120941&action=review Looks good, but a few comments before landing > Tools/MiniBrowser/qt/qml/Dialog.qml:58 > + clip: true Do we need to clip it? > Tools/MiniBrowser/qt/qml/Dialog.qml:64 > + font.weight: Font.Bold Width, and eliding, so that we don't have to clip? > Tools/MiniBrowser/qt/qml/Dialog.qml:69 > + anchors.centerIn: parent Should this have a width and a wrap mode? > Tools/MiniBrowser/qt/qml/Dialog.qml:76 > + anchors.margins: 10 > + anchors.bottom: staticContent.bottom > + anchors.horizontalCenter: staticContent.horizontalCenter I prefer anchors {} when there are more than one property > Tools/MiniBrowser/qt/qml/DialogButton.qml:37 > + color: "#fbfbfb" color: mouseArea.pressed ? .... > Tools/MiniBrowser/qt/qml/DialogButton.qml:57 > + onPressed: button.color = "#eaeaea" You can remove this and use a binding (above) I think we should also include the new qml files in the OTHER_FILES section in MiniBrowser.pro ... Created attachment 120944 [details]
Patch
Comment on attachment 120944 [details] Patch Clearing flags on attachment: 120944 Committed r103940: <http://trac.webkit.org/changeset/103940> All reviewed patches have been landed. Closing bug. |