OpenLP

Merge lp://qastaging/~suutari-olli/openlp/change-blank-to-desktop-hotkey-to-esc into lp://qastaging/openlp

  1. change-blank-to-desktop-hotkey-to-esc
  2. Merge into trunk
Proposed by Azaziah
Status: Merged
Merged at revision: 2699
Proposed branch: lp://qastaging/~suutari-olli/openlp/change-blank-to-desktop-hotkey-to-esc
Merge into: lp://qastaging/openlp
Diff against target: 438 lines (+89/-102)
12 files modified
openlp/core/common/settings.py (+6/-4)
openlp/core/ui/exceptiondialog.py (+1/-1)
openlp/core/ui/exceptionform.py (+1/-1)
openlp/core/ui/mainwindow.py (+16/-32)
openlp/core/ui/shortcutlistform.py (+3/-3)
openlp/core/ui/slidecontroller.py (+35/-30)
openlp/plugins/custom/lib/mediaitem.py (+1/-1)
openlp/plugins/songs/forms/editsongform.py (+1/-1)
resources/images/openlp-2.qrc (+0/-1)
tests/functional/openlp_core_ui/test_slidecontroller.py (+2/-25)
tests/functional/openlp_plugins/bibles/test_mediaitem.py (+21/-0)
tests/functional/openlp_plugins/bibles/test_swordimport.py (+2/-3)
To merge this branch: bzr merge lp://qastaging/~suutari-olli/openlp/change-blank-to-desktop-hotkey-to-esc
Reviewer Review Type Date Requested Status
Tomas Groth Approve
Tim Bentley Approve
Review via email: mp+308660@code.qastaging.launchpad.net

This proposal supersedes a proposal from 2016-10-04.

Description of the change

- Replaced Escape item with "Show desktop"
  (Same as blank to desktop, but does not unblank).
- Combined Offline & Online help buttons into "User Manual" button,
  which launches the appropriate help based on OS. (Offline for Win/Mac)
- Improved blank to modes shortcut descriptions.
- Setting migration for old help/escape help keys.

- Fixed bugs:
https://bugs.launchpad.net/openlp/+bug/805082
https://bugs.launchpad.net/openlp/+bug/1612187
https://bugs.launchpad.net/openlp/+bug/1616441
https://bugs.launchpad.net/openlp/+bug/1497637
https://bugs.launchpad.net/openlp/+bug/1294111

--------------------------------
lp:~suutari-olli/openlp/change-blank-to-desktop-hotkey-to-esc (revision 2716)
[SUCCESS] https://ci.openlp.io/job/Branch-01-Pull/1799/
[SUCCESS] https://ci.openlp.io/job/Branch-02-Functional-Tests/1710/
[SUCCESS] https://ci.openlp.io/job/Branch-03-Interface-Tests/1648/
[SUCCESS] https://ci.openlp.io/job/Branch-04a-Windows_Functional_Tests/1404/
[SUCCESS] https://ci.openlp.io/job/Branch-04b-Windows_Interface_Tests/994/
[SUCCESS] https://ci.openlp.io/job/Branch-05a-Code_Analysis/1062/
[SUCCESS] https://ci.openlp.io/job/Branch-05b-Test_Coverage/930/
[SUCCESS] https://ci.openlp.io/job/Branch-05c-Code_Analysis2/91/

To post a comment you must log in.
Revision history for this message
Azaziah (suutari-olli) wrote : Posted in a previous version of this proposal

See the comment for the right place.

"This used to be in self.main_window.warning_message, where it was causing traceback.

Does it still work?
I'm not exactly sure of what it's supposed to do to be honest.

Is there a reason why this could not be an a separate line?"

Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

See below and tests would be nice.

review: Needs Fixing
Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

Looks good to me!
The only thing that nags me is that pressing Esc will also unblank. In my perception of the world Esc should only exit something - not bring it back!
If Tim and Raoul thinks it fine as it is, feel free to ignore me...

review: Needs Information
Revision history for this message
Azaziah (suutari-olli) wrote : Posted in a previous version of this proposal

> Looks good to me!
> The only thing that nags me is that pressing Esc will also unblank. In my
> perception of the world Esc should only exit something - not bring it back!
> If Tim and Raoul thinks it fine as it is, feel free to ignore me...

Good point which I hadn't thought of before.
Perhaps we should create a new shortkey which would
only Blank to desktop but not unblank?

Revision history for this message
Tomas Groth (tomasgroth) wrote : Posted in a previous version of this proposal

A new blank-only shortcut sounds great to me :-)

Revision history for this message
Phill (phill-ridout) wrote : Posted in a previous version of this proposal

Looks good, just one question (see my in line comment).

Also don't forget to regenerate the resource file and submit it in a separate merge request (after this one has gone in)

Revision history for this message
Phill (phill-ridout) wrote : Posted in a previous version of this proposal

Should note that I've not tested this

Revision history for this message
Azaziah (suutari-olli) wrote : Posted in a previous version of this proposal

> Looks good, just one question (see my in line comment).
>
> Also don't forget to regenerate the resource file and submit it in a separate
> merge request (after this one has gone in)

I've answered the question.

Revision history for this message
Azaziah (suutari-olli) wrote : Posted in a previous version of this proposal

> Should note that I've not tested this

This should probably be tested on Linux just to make sure the Window manager
handles Blank to Desktop in single screen scenarios.

Revision history for this message
Azaziah (suutari-olli) wrote : Posted in a previous version of this proposal

Ended up being unsaved comment, however:

"The automatic wrap puts all of text expect the last word to the same line,
it does not look good at all. I believe manual newline is better in this case.

See the image for reference:

https://drive.google.com/file/d/0B9y8rZiYItltamFQM0hOU0Nhd2s/view?usp=sharing "

Revision history for this message
Phill (phill-ridout) wrote : Posted in a previous version of this proposal

I've spoke to superfly, and he's happy with you adding the \n so looks good to me!

Revision history for this message
Tomas Groth (tomasgroth) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Tim Bentley (trb143) : Posted in a previous version of this proposal
review: Approve
Revision history for this message
Tim Bentley (trb143) wrote : Posted in a previous version of this proposal

Sorry conflicts with trunk.

review: Needs Fixing
Revision history for this message
Tim Bentley (trb143) :
review: Approve
Revision history for this message
Tomas Groth (tomasgroth) :
review: Approve

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
The diff is not available at this time. You can reload the page or download it.