Make the filename placeholders clickable
ClosedPublic

Authored by davidre on Mar 14 2019, 10:49 AM.

Details

Summary

This allows the user to click the placeholders. First step to improve the filename template configuration. I also moved the available placeholders to the ExportManager. The SaveOptionsPage doesn't need to care about which options are present and we have to change them in only one class if we want to make changes.
Further steps I plan to do are displaying a preview, improving the Layout (we had to increase the size already) and eventually using better placeholders.

BUG: 390856
FIXED-IN: 19.04.0

Test Plan
  • Click on placeholder
  • It appears in the LineEdit

Diff Detail

Repository
R166 Spectacle
Branch
nametemplate
Lint
No Linters Available
Unit
No Unit Test Coverage
Build Status
Buildable 9573
Build 9591: arc lint + arc unit
davidre created this revision.Mar 14 2019, 10:49 AM
Restricted Application added a project: Spectacle. · View Herald TranscriptMar 14 2019, 10:49 AM
davidre requested review of this revision.Mar 14 2019, 10:49 AM
davidre edited the summary of this revision. (Show Details)Mar 14 2019, 11:09 AM

Sweet! This works great! Nice code refactoring, too.

Could we avoid making these a bulleted list, though? The bullet points are a bit distracting.

Will do. I just copied the bullet list from the Gwenview importer ;D

davidre updated this revision to Diff 53890.Mar 14 2019, 3:29 PM

Don' use bullet list, go back to blockquote approach

Sweet, thanks! IMO we should change that in Gwenview too. The bullets are distracting to my eye.

Code looks good. I just spotted one last little issue, and then let's get this in. :)

src/Gui/SettingsDialog/SaveOptionsPage.cpp
135

Forgot a space in this string between the character and the explanatory text.

davidre updated this revision to Diff 53896.Mar 14 2019, 5:22 PM
  • Add missing space
davidre marked an inline comment as done.Mar 14 2019, 5:22 PM
ngraham added inline comments.Mar 14 2019, 5:25 PM
src/ExportManager.cpp
523

Sorry, one last thing: "minute" needs to be capitalized so it matches all the others.

davidre updated this revision to Diff 53898.Mar 14 2019, 5:29 PM
  • Minute
davidre marked an inline comment as done.Mar 14 2019, 5:29 PM
ngraham accepted this revision.Mar 14 2019, 5:32 PM
ngraham edited the summary of this revision. (Show Details)

Thanks, this is great!

This revision is now accepted and ready to land.Mar 14 2019, 5:32 PM
This revision was automatically updated to reflect the committed changes.