Differential D11374

QShortcut: replace 'clickFunc()' with lambda; rename variable to 'takeScreenshotShortcut'
ClosedPublic
Actions

Authored by sharvey on Mar 16 2018, 2:43 AM.

Details

Reviewers
rkflx
ngraham
Group Reviewers
Spectacle
Commits
R166:43306c60bcfc: QShortcut: replace 'clickFunc()' with lambda; rename variable to…
Summary

Removed clickFunc() entirely; replaced with lambda. Renamed QShortcut variable
to takeScreenshotShortcut for clarity. Eliminated redundant QTimer::singleShot function call,
as animateClick() emits all the necessary signals on its own.

Test Plan
  • Launch Spectacle in GUI mode
  • Take screenshot by pressing Enter (default GUI key)
  • Take screenshot by pressing CTRL+N ("new document" shortcut)

Diff Detail

Repository
R166 Spectacle
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sharvey requested review of this revision.Mar 16 2018, 2:43 AM
sharvey created this revision.
sharvey edited the test plan for this revision. (Show Details)Mar 16 2018, 2:50 AM
sharvey edited the summary of this revision. (Show Details)Mar 16 2018, 2:52 AM
sharvey edited the summary of this revision. (Show Details)Mar 16 2018, 2:54 AM
rkflx requested changes to this revision.Mar 17 2018, 8:58 AM

Thanks! Nearly perfect, just some minor formatting left…

src/Gui/KSWidget.cpp
130

I would name this takeScreenshotShortcut, because the "default" behaviour is actually a property of the button, and with the added "shortcut" it reads a bit more naturally in other places.

131

The formatting is usually done similar to a function:

connect(takeScreenshotShortcut, &QShortcut::activated, [this]() {
    mTakeScreenshotButton->animateClick(100);
});
This revision now requires changes to proceed.Mar 17 2018, 8:58 AM
sharvey updated this revision to Diff 29744.Mar 17 2018, 10:19 AM
sharvey edited the test plan for this revision. (Show Details)
  • Replaced clickFunc with lambda; renamed shortcut variable

Reformatted lambda to customary style (split over multiple lines)

sharvey retitled this revision from QShortcut: replace 'clickFunc()' with lambda function; rename variable to 'defaultNew' to QShortcut: replace 'clickFunc()' with lambda; rename variable to 'takeScreenshotShortcut'.Mar 17 2018, 10:22 AM
sharvey edited the summary of this revision. (Show Details)
sharvey added a comment.Mar 17 2018, 10:28 AM

Thanks! Nearly perfect, just some minor formatting left…

That formatting is much clearer. No more pile of braces, semicolons, and parentheses at the end of the statement. QtCreator does a passable job at highlighting matching brackets, but I'm still trying to get used to it. At times, it feels like I'm right-handed, trying to use a left-handed tool. I guess I'll have to become multi-talented, as I can't afford the $200/year for CLion. I've tried KDevelop, but if I have to learn something new, it may as well be QtCreator so I can take advantage of the included layout design tools.

This has been another good learning experience. Thank you for sharing your knowledge.

rkflx accepted this revision.Mar 17 2018, 11:14 PM

Thanks, looks great now. You even improved the commit message.

I can't afford the $200/year for CLion

Personally I would not want to use such bloated and proprietary software, I don't think you are missing out.

This revision is now accepted and ready to land.Mar 17 2018, 11:14 PM
Closed by commit R166:43306c60bcfc: QShortcut: replace 'clickFunc()' with lambda; rename variable to… (authored by sharvey, committed by rkflx). · Explain WhyMar 17 2018, 11:15 PM
This revision was automatically updated to reflect the committed changes.

Revision Contents
Changeset List

PathPackages
Msrc/Gui/KSWidget.cpp (8 lines)

Diff 29802

View Options

src/Gui/KSWidget.cpp

Loading...