Differential D16714

[Terminal Display] Don't hide the search box when scrolling with the keyboard
ClosedPublic
Actions

Authored by thsurrel on Nov 6 2018, 5:30 PM.

Details

Reviewers
anthonyfieroni
Group Reviewers
Konsole
Commits
R319:1c4f6818d676: [Terminal Display] Don't hide the search box when scrolling with the keyboard
Summary

When hitting Shift-Scrollup while having the search box opened, the search
box is closed.

Regression introduced by f93263d6b4dca25416b17c96ac9a996ee2059ef6

Test Plan

Open the search box, then scroll with the keyboard (e.g. with Shift-Scrollup)
The searchbox should not close.

Diff Detail

Repository
R319 Konsole
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
thsurrel created this revision.Nov 6 2018, 5:30 PM
Restricted Application added a project: Konsole. · View Herald TranscriptNov 6 2018, 5:30 PM
Restricted Application added a subscriber: konsole-devel. · View Herald Transcript
thsurrel requested review of this revision.Nov 6 2018, 5:30 PM
anthonyfieroni added a subscriber: anthonyfieroni.Nov 6 2018, 6:18 PM

Shift + Pg Up/Down works for me and why it will set up esc key?

thsurrel added a comment.EditedNov 6 2018, 8:41 PM

According to http://doc.qt.io/qt-5/qt.html#Key-enum, Qt::Key_Escape is 0x01000000 and Qt::Key_PageUp (for example) is 0x01000016. So the condition event->key() & Qt::Key_Escape will be true when hitting Shift-pageup to scroll and ends up closing the searchbox.

thsurrel added a comment.Nov 6 2018, 9:59 PM

And the bug is only in master, maybe this is why you did not see it ?

ngraham added a subscriber: ngraham.Nov 7 2018, 4:24 AM

Thanks, this works great for me.

anthonyfieroni accepted this revision.Nov 7 2018, 6:23 AM
This revision is now accepted and ready to land.Nov 7 2018, 6:23 AM
Closed by commit R319:1c4f6818d676: [Terminal Display] Don't hide the search box when scrolling with the keyboard (authored by thsurrel). · Explain WhyNov 7 2018, 8:17 AM
This revision was automatically updated to reflect the committed changes.
hindenburg added a subscriber: hindenburg.Nov 7 2018, 2:38 PM

As a FYI, try to keep the lines < 70 columns if you can. It is annoying that phabricator doesn't give any indications of columns. If you use > 70, it makes the commit message and body multi-line.

thsurrel added a comment.Nov 7 2018, 2:40 PM

Oh, I did not know that. I will definitely pay attention in the future, sorry.

Revision Contents
Changeset List

PathPackages
Msrc/TerminalDisplay.cpp (2 lines)

Diff 45006

View Options

src/TerminalDisplay.cpp

Loading...