Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix #105716 Album option Page/Section Break #2536

Merged
merged 1 commit into from
Apr 15, 2016

Conversation

ericfont
Copy link
Contributor

No description provided.

@ericfont ericfont force-pushed the 105716-AlbumManager-PageBreak branch from b3cae6f to 47a1241 Compare April 11, 2016 22:28
@@ -196,7 +244,6 @@
<tabstop>createNew</tabstop>
<tabstop>load</tabstop>
<tabstop>print</tabstop>
<tabstop>createScore</tabstop>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why taking it out of the tabstops? I guess instead checkBoxAddPageBreak and checkBoxAddSectionBreak should get added.

@ericfont ericfont force-pushed the 105716-AlbumManager-PageBreak branch 2 times, most recently from 9f848b1 to e1d436f Compare April 12, 2016 16:18
@ericfont
Copy link
Contributor Author

@Jojo-Schmitz I've re-added Join Scores to tabstop and added checkBoxAddSectionBreak checkBoxAddPageBreak to tabstop.

Thanks for catching that. I guess when I moved the button inside the QGroupBox, the QtCreator ui editor unintentionally removed the tabstop.

lastMeasure->undoSetBreak(true, LayoutBreak::Type::LINE);
if (addPageBreak) {
// add PageBreak if one doesn't already exist
if (!lastMeasure->pageBreak())
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm looking over this line and am thinking maybe I should do

if (!lastMeasure->lineBreak() && !lastMeasure->pageBreak())
instead, so that only a Line or Page break will be the result, not both Line & Page break.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So score that have a line break at their end, where this presumably has been added on purpose, don't get a page break. Yes, sounds good to me.

OTOH if the user specified 'add page break' in the dialog (s)he should get it, shouldn't (s)he?
But you can't have a line break and a page break at the same place, at least not via the palette, so if anything, the page break should *replace * the line break.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good point...I should modify this so that Page Break will replace preexisting Line Break.

@Jojo-Schmitz
Copy link
Contributor

rebase needed

@ericfont ericfont force-pushed the 105716-AlbumManager-PageBreak branch 2 times, most recently from 4b67734 to c1c75fe Compare April 14, 2016 19:59
@@ -1761,13 +1761,17 @@ bool Score::appendScore(Score* score)
return false;
int tickOfAppend = lastMeasure->endTick();

if (!lastMeasure->lineBreak() && !lastMeasure->pageBreak()) {
lastMeasure->undoSetBreak(true, LayoutBreak::Type::LINE);
// apply Page/Section Breaks if desired
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Jojo-Schmitz, I've changed around this logic regarding addPageBreak so is as follows. If adding a pagebreak, will first remove a line break if one exists. If not adding a page break, will simply add a line break (if a page break or line break doesn't already exist)...this is same behavior pre-PR.

@ericfont
Copy link
Contributor Author

(rebased).

@lasconic lasconic merged commit 1cf5caf into musescore:master Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants