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

[MU3] fix #296528: Footers now appear inside page margins #5902

Closed
wants to merge 1 commit into from

Conversation

SKefalidis
Copy link
Contributor

Resolves: https://musescore.org/en/node/296528

Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

  • I signed CLA
  • I made sure the code in the PR follows the coding rules
  • I made sure the code compiles on my machine
  • I made sure there are no unnecessary changes in the code
  • I made sure the title of the PR reflects the core meaning of the issue you are solving
  • I made sure the commit message(s) contain a description and answer the question "Why do those changes fix that particular issue?" or "Why are those changes really necessary as improvements?"
  • I made sure the commit message title starts with "fix #424242:" if there is a related issue
  • [n\a] I created the test (mtest, vtest, script test) to verify the changes I made

@dmitrio95
Copy link
Contributor

As mentioned also in the issue, this seems to effectively reverse the change made in #4944, although the implementation seems to be different.

@igorkorsukov igorkorsukov changed the title fix #296528: Footers now appear inside page margins [MU4] fix #296528: Footers now appear inside page margins Feb 5, 2021
@SKefalidis SKefalidis changed the base branch from master to 3.x March 3, 2021 14:13
@SKefalidis SKefalidis changed the title [MU4] fix #296528: Footers now appear inside page margins [MU3] fix #296528: Footers now appear inside page margins Mar 3, 2021
@SKefalidis
Copy link
Contributor Author

Updated and rebased. Added the option of how to draw the footers as a style.

image

@Jojo-Schmitz
Copy link
Contributor

Adds one string to the translations. Not an issue, just noting...

@RobFog
Copy link

RobFog commented Mar 3, 2021

@dmitrio95 What do you think makes more sense?

@dmitrio95
Copy link
Contributor

Well, I am not that sure what makes more sense. The change made by this PR makes both footers and headers lie outside of the margins area which is indeed more consistent than the situation we have now. In this regard I think this change makes sense.

As this PR introduces a new style setting, footerInsideMargins, it may also make sense for compatibility reasons to set it to true when importing 2.X and perhaps older 3.X scores (this seems to be determined now by the files in share/styles). Otherwise this change, while fixing the inconsistent default settings for newer scores, also effectively returns this issue back.

@Jojo-Schmitz
Copy link
Contributor

I can't see any effect when toggling that option "Inside margins", it still stays outside the page margins?

Seems 2.x has it outside the margins, (2.3.2 at least). 3.5.2 too (and I have no older 3.x handy to test), so no need to tweak the style files.

@Jojo-Schmitz
Copy link
Contributor

I take it all back, I just accidentally left out one line of you PR, and of course (Murphy's Law) the vital one from textbase.cpp...
It does seem to place it way too high though?

Still no style default change needed for any prior version as far as I can tell. Or would this be needed only needed for 3.0?

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 4, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 4, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 4, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 4, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 4, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 9, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 10, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 23, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 24, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 26, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Sep 29, 2021
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request May 12, 2022
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
@RomanPudashkin
Copy link
Contributor

3.x is closed for any changes

Jojo-Schmitz pushed a commit to Jojo-Schmitz/MuseScore that referenced this pull request Mar 5, 2023
Added the ability to layout a textbase using the bottom margin (the old way used the top margin in every case). This change fixes the issue.

Duplicate of musescore#5902
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

5 participants