[Webkit-unassigned] [Bug 74074] [Qt] Visualize mock points in the Qt MiniBrowser

[bugzilla-daemon at webkit.org]

https://bugs.webkit.org/show_bug.cgi?id=74074





--- Comment #3 from Tor Arne Vestbø <vestbo at webkit.org>  2011-12-08 05:21:20 PST ---
(From update of attachment 118363)
View in context: https://bugs.webkit.org/attachment.cgi?id=118363&action=review

> Tools/MiniBrowser/qt/BrowserWindow.cpp:86
> +void BrowserWindow::updateTouchMockPoint(int id, const QPointF& position, qreal opacity)

updateMockTouchPoints

>> Tools/MiniBrowser/qt/BrowserWindow.cpp:88
>> +    QHash<int, QQuickItem*>::iterator iter = m_touchMockPoints.find(id);
> 
> we normally use the name "it" in webkit

Look up old touch points that you can re-use from the engine based on the id, instead of keeping a list.

> Tools/MiniBrowser/qt/BrowserWindow.cpp:97
> +    iter.value()->setX(position.x() / 2);

set the x and y, and do the offsetting to center it in the qml

>> Tools/MiniBrowser/qt/BrowserWindow.cpp:99
>> +    iter.value()->setOpacity(opacity);
> 
> why not (*it)->set...

i'd rather see a "pressed" property that you set and unset based on the point being released or not, and you can then deal with how you hide it in the qml

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:163
> +

do we ever have a situation without a browser window?

> Tools/MiniBrowser/qt/MiniBrowserApplication.cpp:168
> +        qreal opacity = 1.0;

see comment about "pressed". we should not deal with opacity here, that's an implementation detail of the visual component

-- 
Configure bugmail: https://bugs.webkit.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the assignee for the bug.