Skip to content

Fix for #8964: Properties Panel empty when no score open - #17278

Merged
Eism merged 1 commit into
musescore:masterfrom
AnnikaLevesque:properties-no-score
May 31, 2023
Merged

Fix for #8964: Properties Panel empty when no score open#17278
Eism merged 1 commit into
musescore:masterfrom
AnnikaLevesque:properties-no-score

Conversation

@AnnikaLevesque

Copy link
Copy Markdown
Contributor

Resolves: #8964

In response to #8964 (comment), the modules in the Properties Panel that have no effect when no score is open will no longer appear when no score is open. They will appear when a score is opened and nothing is selected, and disappear again if (all) score(s) are closed.

  • I signed the CLA
  • The title of the PR describes the problem it addresses
  • Each commit's message describes its purpose and effects, and references the issue it resolves
  • If changes are extensive, there is a sequence of easily reviewable commits
  • The code in the PR follows the coding rules
  • There are no unnecessary changes
  • The code compiles and runs on my machine, preferably after each commit individually
  • I created a unit test or vtest to verify the changes I made (if applicable)

I built RUN_TESTS, and the only failing tests seemed to be unrelated ones, such as audio tests and braille tests.

@AnnikaLevesque

Copy link
Copy Markdown
Contributor Author

I can't figure out how to run uncrustify, any help would be appreciated with that (or just let me know what the style issue is)

@Jojo-Schmitz

Jojo-Schmitz commented Apr 19, 2023

Copy link
Copy Markdown
Contributor

You don't really need to run uncrustify yourself, just make the code changes proposed by the codestyle check

diff --git a/src/inspector/models/inspectorlistmodel.cpp b/src/inspector/models/inspectorlistmodel.cpp
index 82ab4bc..68d80f2 100644
--- a/src/inspector/models/inspectorlistmodel.cpp
+++ b/src/inspector/models/inspectorlistmodel.cpp
@@ -70,7 +70,7 @@ void InspectorListModel::buildModelsForEmptySelection()
         removeUnusedModels({}, false /*isRangeSelection*/);
         return;
     }
-    
+
     removeUnusedModels({}, false /*isRangeSelection*/, persistentSectionList);
 
     createModelsBySectionType(persistentSectionList);

It basically is a tab or a couple spaces too much

@RomanPudashkin
RomanPudashkin requested a review from Eism May 10, 2023 09:23
Comment thread src/inspector/models/inspectorlistmodel.cpp
Comment thread src/inspector/models/inspectorlistmodel.cpp
@Eism

Eism commented May 11, 2023

Copy link
Copy Markdown
Collaborator

Also please squash two commits about codestayle into other commits where you did these changes

@AnnikaLevesque

Copy link
Copy Markdown
Contributor Author

Thank you for taking a look at it! I'm currently travelling, will fix when I get back to my computer.

@AnnikaLevesque
AnnikaLevesque marked this pull request as draft May 24, 2023 20:23
AnnikaLevesque added a commit to AnnikaLevesque/MuseScore that referenced this pull request May 24, 2023
AnnikaLevesque added a commit to AnnikaLevesque/MuseScore that referenced this pull request May 24, 2023
AnnikaLevesque added a commit to AnnikaLevesque/MuseScore that referenced this pull request May 24, 2023
Whenever the "Properties" panel is being rebuilt because nothing is selected, it checks if a score is open, and if not, then the Properties panel doesn't build any rows (i.e. it stays empty). This check triggers whenever the selected element changes from 'something' to 'nothing'.

codestyle fix

I didn't realize that there was a default argument for removeUnusedModels, now it's using it

musescore#17278 (comment)
@AnnikaLevesque
AnnikaLevesque marked this pull request as ready for review May 24, 2023 21:51
@DmitryArefiev

Copy link
Copy Markdown
Contributor

@zacjansheski Hi! Can you test this on Mac and Win please?

@zacjansheski

Copy link
Copy Markdown
Contributor

Testing on Mac OS. Please check that the "eyes" under "visible" are not affected by this change.

Screen shot attached showing property window on new score:
PR - on the left. (all eyes are closed)
4.0.2 - on the right

Screen Shot 2023-05-25 at 1 02 43 PM

@AnnikaLevesque

AnnikaLevesque commented May 25, 2023

Copy link
Copy Markdown
Contributor Author

It seems that the "Invisible", "Formatting", and "Frames" eyes have behavior that is the opposite of what is expected (those elements become visible when the eye icon is closed and vice versa), but I don't know why this is the case. If I select something on the score and return to empty selection, the eye icons go back to behaving normally. If I close the score and reopen a new one, they are wrong again.

Is there something unique that happens when these panels are populated for the first time or when they are populated with no score? Is the "Page margins" eye different? I don't know why that one seems to work when the others don't.

@zacjansheski

zacjansheski commented May 25, 2023

Copy link
Copy Markdown
Contributor

It's notable that when the properties panel is visible in the nightly (with no score opened), all eyes are shut, so I think you are correct in seeing there is change happening here when a score is opened (all but "Page Margins" are flipped to open)

Screen Shot 2023-05-25 at 2 55 08 PM

@AnnikaLevesque

AnnikaLevesque commented May 25, 2023

Copy link
Copy Markdown
Contributor Author

Re: the last commit, I'm guessing that when a new score is opened (or the panel is created for the first time), each "model" in the properties panel is created with the constructor, but subsequent uses of the model just updates it instead of creating a new one. This doesn't explain why it works properly on the nightly build though, so I think I'm still missing something.

@ghost

ghost commented May 26, 2023

Copy link
Copy Markdown

@AnnikaLevesque Could you please rebase this PR? That may (should) solve the mysterious failing vtests.

Whenever the "Properties" panel is being rebuilt because nothing is selected, it checks if a score is open, and if not, then the Properties panel doesn't build any rows (i.e. it stays empty). This check triggers whenever the selected element changes from 'something' to 'nothing'.
@zacjansheski

Copy link
Copy Markdown
Contributor

Tested on MacOS 11.6.5, Ubuntu 20.04.6 LTS, and Windows 11. Works well!

@Eism
Eism merged commit 87ea144 into musescore:master May 31, 2023
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.

[MU4 Issue] Palettes/Properties panel fully enabled even when no score open

5 participants