SSIA
Created attachment 114510 [details] Patch
Comment on attachment 114510 [details] Patch looks awesome to me :)
Created attachment 114515 [details] Rebased
Comment on attachment 114515 [details] Rebased View in context: https://bugs.webkit.org/attachment.cgi?id=114515&action=review r=me with some cool stories, bro. > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:251 > + if (scrollAnimationActive() || scaleAnimationActive()) > + return; > + > + if (!targetArea.isValid()) > + return; Nit: I'd put the !targetArea.isValid() check first, since it's inline. > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:253 > + int margin = 10; // We want at least a little bit or margin. This variable should be const. > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:273 > + QRectF endPosRange = computePosRangeForItemAtScale(endItemScale); endPositionRange would be a better name I think. Actually, thinking about it, I personally prefer "final" to "end" in this context, but maybe "end" is an existing QtQuick convention. Regardless, NABD.
(In reply to comment #4) > (From update of attachment 114515 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=114515&action=review > > r=me with some cool stories, bro. > > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:251 > > + if (scrollAnimationActive() || scaleAnimationActive()) > > + return; > > + > > + if (!targetArea.isValid()) > > + return; > > Nit: I'd put the !targetArea.isValid() check first, since it's inline. Good point > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:253 > > + int margin = 10; // We want at least a little bit or margin. > > This variable should be const. Sure > > Source/WebKit2/UIProcess/qt/QtViewportInteractionEngine.cpp:273 > > + QRectF endPosRange = computePosRangeForItemAtScale(endItemScale); > > endPositionRange would be a better name I think. It is, but Pos was choosed to be similar to the QQuick API :-) setPosRange() *hint* > Actually, thinking about it, I personally prefer "final" to "end" in this context, but maybe "end" is an existing QtQuick convention. Regardless, NABD. Yeah that is used all over in the API (and in our code)
Comment on attachment 114515 [details] Rebased Landed in 100125