trmdi added a comment.
In D16643#406076 <https://phabricator.kde.org/D16643#406076>, @bcooksley
wrote:
> Please update your name on Identity, then i'll sync it from there.
Done.
Thank you!
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.
trmdi added a comment.
Nice, my first patch finally has been accepted.
Thanks everyone!
Btw, can you @bcooksley help me to change my name in my profile? Tranter Madi
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma
trmdi added a comment.
In D16643#406051 <https://phabricator.kde.org/D16643#406051>, @fvogt wrote:
> @trmdi: Do you have push access? If not, which name should be used
for the commit?
No, I don't.
tr...@yandex.com / Tranter Madi
REPOSITORY
R296 KDeclarative
R
trmdi updated this revision to Diff 50979.
REPOSITORY
R296 KDeclarative
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16643?vs=47815&id=50979
REVISION DETAIL
https://phabricator.kde.org/D16643
AFFECTED FILES
src/qmlcontrols/draganddrop/DeclarativeDropArea.cpp
To: trmdi,
trmdi added a comment.
But wait, can we completely remove `m_oldDragMovePos` ?
If the event is accepted in the case `event->pos() == m_oldDragMovePos`, then
why don't we generate the move event?
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
T
trmdi added a comment.
@broulik
I've updated the patch, can you review it ?
In D16643#378823 <https://phabricator.kde.org/D16643#378823>, @broulik wrote:
> Only if that potential issue with inhibition has been addressed
REPOSITORY
R296 KDeclarative
REVISION
trmdi added a comment.
!ping
REPOSITORY
R242 Plasma Framework (Library)
REVISION DETAIL
https://phabricator.kde.org/D15839
To: mvourlakos, #plasma, davidedmundson, mart
Cc: trmdi, kde-frameworks-devel, michaelh, ngraham, bruns
trmdi edited the summary of this revision.
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, bruns
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi added a comment.
What will happen if @bruns doesn't want to reply here anymore?
I've updated the patch, can you review it ? @broulik
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, bruns
Cc: anth
trmdi added a comment.
!ping
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, bruns
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi updated this revision to Diff 47815.
trmdi edited reviewers, added: bruns; removed: davidedmundson.
REPOSITORY
R296 KDeclarative
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16643?vs=44860&id=47815
REVISION DETAIL
https://phabricator.kde.org/D16643
AFFECTED FILES
trmdi added a comment.
Is it better if I change the line 91 to `event->setAccepted(m_enabled &&
!m_temporaryInhibition)` @bruns ?
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: ant
trmdi added a comment.
In D16643#378823 <https://phabricator.kde.org/D16643#378823>, @broulik wrote:
> Only if that potential issue with inhibition has been addressed
You mean @bruns is right and I'm wrong?
REPOSITORY
R296 KDeclarative
REVISION
trmdi added a comment.
!ping @hein
Could you land this patch?
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi added a comment.
In D16643#370642 <https://phabricator.kde.org/D16643#370642>, @bruns wrote:
> This should not be accepted as is, it breaks the temporaryInhibition case
when a DropArea is child of another
Ok, could you write a simple QML file to show that case?
R
trmdi added a comment.
I don't know what to do next, can you help me @ngraham ?
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: anthonyfieroni, bruns, dkorth, ngraham, kde-frameworks-
trmdi added a comment.
In D16643#355965 <https://phabricator.kde.org/D16643#355965>, @bruns wrote:
> If it does not receive any events, it can and must not accept. All events
go to the parent.
`DeclarativeDropArea::dragMoveEvent(QDragMoveEvent *event)` is called only
trmdi added a comment.
In D16643#355177 <https://phabricator.kde.org/D16643#355177>, @bruns wrote:
> You have a conceptual misunderstanding of the enabled property, see
QQuickItem <http://doc.qt.io/qt-5/qml-qtquick-item.html#enabled-prop>
>
> > enabled : b
trmdi added a comment.
!Ping Reviewers
Please review this. It's just a very simple patch.
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: bruns, dkorth, ngraham, kde-frameworks-
trmdi added a comment.
In D16643#354120 <https://phabricator.kde.org/D16643#354120>, @bruns wrote:
> `m_enabled` may change between //consecutive// DragMove events, but not
during an event.
>
> In this case, though, it does not, and is constantly true. You can
trmdi updated this revision to Diff 44860.
trmdi added a comment.
git diff -U -- DeclarativeDropArea.cpp > patch.diff
REPOSITORY
R296 KDeclarative
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D16643?vs=44784&id=44860
REVISION DETAIL
https://phabricator.kde.org
trmdi edited the summary of this revision.
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi edited the summary of this revision.
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi added a comment.
@ngraham
How to see the full file content from this page?
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc: bruns, dkorth, ngraham, kde-frameworks-devel, michaelh
trmdi added a comment.
In D16643#353670 <https://phabricator.kde.org/D16643#353670>, @bruns wrote:
> This is not covered by your summary - you have only listed cases where the
drag is erroneously **not** accepted. Please update the summary.
I wrote it in the f
trmdi added a comment.
In D16643#353670 <https://phabricator.kde.org/D16643#353670>, @bruns wrote:
> No. The event loop is running in a single thread. m_enabled is constant
during the function.
Can it be changed from outside?
REPOSITORY
R296 KDeclarative
REVISI
trmdi added a comment.
@bruns
If `setAccepted` goes below the first if:
- if there is a change that make m_enabled change from true -> false while
moving, the cursor still display the dragable icon
So I let it be on the top to make the cursor be able to change the icon
trmdi retitled this revision from "Correct m_enabled on DragMove " to "Correct
the accept flag of the event object on DragMove ".
REPOSITORY
R296 KDeclarative
REVISION DETAIL
https://phabricator.kde.org/D16643
To: trmdi, mart, broulik, #plasma, hein, davidedmundson
Cc:
trmdi added a comment.
In D16643#353517 <https://phabricator.kde.org/D16643#353517>, @ngraham wrote:
> Wow, does this really fix 396011!? If so, congratulations! I wish I could
test, but alas (or thankfully?) I don't experience the problem.
Yes, it fixes the proble
trmdi created this revision.
trmdi added a reviewer: mart.
Herald added a project: Frameworks.
Herald added a subscriber: kde-frameworks-devel.
trmdi requested review of this revision.
REVISION SUMMARY
- `m_enabled` could change while moving.
- Don't call `setAccepted(false)` wrong
30 matches
Mail list logo
