rjvbb added a comment.
Thanks.
I've been using this patch myself and never noticed any side-effects. Except
when an application shows to be susceptible to accidental zooming ... because
it uses another widget class that doesn't have this protection :)
REPOSITORY
R39 KTextEditor
This revision was automatically updated to reflect the committed changes.
Closed by commit R39:f51befdc80b1: prevent accidental zooming. (authored by
rjvbb).
CHANGED PRIOR TO COMMIT
https://phabricator.kde.org/D5037?vs=15747=22232#toc
REPOSITORY
R39 KTextEditor
CHANGES SINCE LAST UPDATE
cullmann added a comment.
Given people seem to have tested this, I would be ok to let it go in now.
I guess we need some "user-test" for it anyways.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: cullmann, ngraham, kfunk,
ngraham edited the summary of this revision.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: ngraham, kfunk, luebking, anthonyfieroni, dhaumann, kwrite-devel,
#ktexteditor, #frameworks, head7, cullmann, sars
ngraham added a comment.
What are the next steps here?
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: ngraham, kfunk, luebking, anthonyfieroni, dhaumann, kwrite-devel,
#ktexteditor, #frameworks, head7, cullmann, sars
rjvbb updated this revision to Diff 15747.
rjvbb marked 2 inline comments as done.
rjvbb added a comment.
Updated as requested.
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5037?vs=15727=15747
REVISION DETAIL
https://phabricator.kde.org/D5037
AFFECTED FILES
rjvbb set the repository for this revision to R39 KTextEditor.
REPOSITORY
R39 KTextEditor
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: kfunk, luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor,
#frameworks, head7, cullmann, sars
rjvbb marked 6 inline comments as done.
rjvbb added inline comments.
INLINE COMMENTS
> kfunk wrote in kateviewinternal.cpp:87
> Simplify: `else if (...) {`?
Why the question mark? Simplification should be perfectly fine here, no?
> kfunk wrote in kateviewinternal.cpp:121
> Initialize here
kfunk added a comment.
A couple of minor issues I spotted while looking briefly over the patch.
INLINE COMMENTS
> kateviewinternal.cpp:76
> +
> +bool detectZoomingEvent(QWheelEvent *e, Qt::KeyboardModifiers
> modifier=Qt::ControlModifier)
> +{
Style: Use ` = `
>
rjvbb updated this revision to Diff 15727.
rjvbb added a comment.
refreshed for git/head (and ¡bump!)
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5037?vs=12608=15727
REVISION DETAIL
https://phabricator.kde.org/D5037
AFFECTED FILES
src/view/kateviewinternal.cpp
rjvbb updated this revision to Diff 12608.
rjvbb added a comment.
Updated as requested
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5037?vs=12480=12608
REVISION DETAIL
https://phabricator.kde.org/D5037
AFFECTED FILES
src/view/kateviewinternal.cpp
dhaumann added a comment.
Yes, this is what I meant. I think the code is now better by using an
isolated class.
I just read the code again, and I think I would like to suggest another
change: The class name currently is too generic, this makes reasoning quite
hard.
That said, I
rjvbb updated this revision to Diff 12480.
rjvbb added a comment.
- don't use QElapsedTimer::elapsed() when the timer isn't valid. Changes
nothing to the behaviour but may make the code a bit less obfuscated.
- edited a few comments
CHANGES SINCE LAST UPDATE
rjvbb added a comment.
In https://phabricator.kde.org/D5037#95031, @anthonyfieroni wrote:
> Ok, note this situation, hold modifier before first ever wheel event => you
call m_lastWheelEvent.elapsed on unstarted timer
This is true, and things can indeed be improved a bit there.
rjvbb added a subscriber: luebking.
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: luebking, anthonyfieroni, dhaumann, kwrite-devel, #ktexteditor,
#frameworks, head7, cullmann, kfunk, sars
rjvbb added a comment.
In https://phabricator.kde.org/D5037#94944, @anthonyfieroni wrote:
> Ok, we discard first wheel event cause elapsed timer isn't started, right?
No, it's not discarded; no wheel events are being discarded unless they
already were being discarded. What my
anthonyfieroni added a comment.
Ok, we discard first wheel event cause elapsed timer isn't started, right? I
think we can "catch" all events we just make it a bit different, set modifiers
in contructor and restart elpased timer on global event override function if
desired modifiers
rjvbb updated this revision to Diff 12466.
rjvbb added a comment.
This makes unsetting the modifier non-optional and rewords some comments
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5037?vs=12448=12466
REVISION DETAIL
https://phabricator.kde.org/D5037
AFFECTED FILES
rjvbb marked an inline comment as done.
rjvbb added inline comments.
INLINE COMMENTS
> anthonyfieroni wrote in kateviewinternal.cpp:76
> About me, it's designed to unset modifiers so param looks unwanted.
I don't disagree but not for exactly the same reason. I'm not sure the function
is or
anthonyfieroni added inline comments.
INLINE COMMENTS
> kateviewinternal.cpp:76
> +
> +bool detectZoomingEvent(QWheelEvent *e, bool unsetModifier=true,
> Qt::KeyboardModifiers modifier=Qt::ControlModifier)
> +{
About me, it's designed to unset modifiers so param looks unwanted.
>
rjvbb marked 2 inline comments as done.
REVISION DETAIL
https://phabricator.kde.org/D5037
To: rjvbb, #ktexteditor
Cc: dhaumann, kwrite-devel, #ktexteditor, #frameworks, head7, cullmann, kfunk,
sars
rjvbb updated this revision to Diff 12448.
rjvbb edited the test plan for this revision.
rjvbb added a comment.
Was this what you had in mind?
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D5037?vs=12438=12448
REVISION DETAIL
https://phabricator.kde.org/D5037
AFFECTED FILES
dhaumann added a comment.
Thinking aloud: Can you factor this out into a small helper class called
WheelEventFilter or similar? I can imagine this would be cleaner.
The code possibly would shrink down to something like:
if (m_wheelEventFilter.processEvent(event)) {
// ...
rjvbb created this revision.
rjvbb added a project: KTextEditor.
Restricted Application added subscribers: Frameworks, kwrite-devel.
Restricted Application added a project: Frameworks.
REVISION SUMMARY
KTextEditor::KateViewInternal::wheelEvent() implements text zoom in reaction
to Ctrl+Wheel
24 matches
Mail list logo
