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 #316571 updated close icon file reference for MIDI import panel #7363

Closed
wants to merge 1 commit into from

Conversation

jeetee
Copy link
Contributor

@jeetee jeetee commented Jan 29, 2021

Opted for the toolbar- version over the dialog- version due to better matching size with the arrows

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

got overlooked in #7072

  • 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

Opted for the toolbar- version over the dialog- version due to better matching size with the arrows
@@ -83,7 +83,7 @@
</property>
<property name="icon">
<iconset resource="../musescore.qrc">
<normaloff>:/data/icons/png/window-close.png</normaloff>:/data/icons/png/window-close.png</iconset>
<normaloff>:/data/icons/png/toolbar-close.png</normaloff>:/data/icons/png/toolbar-close.png</iconset>
Copy link
Contributor

Choose a reason for hiding this comment

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

But where is toolbar-close.png? It doesn't seem to exist, and in the artifact the icon doesn't show up for me. I think it would be the best to take :/data/icons/window-close.svg.

Futhermore, in mscore/importmidi_ui/importmidi_panel.cpp you need two add these lines:

+      _ui->toolButtonHideMidiPanel->setIcon(*icons[int(Icons::close_ICON)]);
+      _ui->pushButtonUp->setIcon(*icons[int(Icons::arrowUp_ICON)]);
       _ui->pushButtonDown->setIcon(*icons[int(Icons::arrowDown_ICON)]);

And in mscore/data/icons/window-close.svg:

- <svg id="Layer_1" data-name="Layer 1" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48">
+ <svg fill="#3B3F45" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 48 48">

This is needed to get it to work correctly with dark mode. However, switching between light and dark mode won't work, like in very many places in the program. That would be quite complicated to solve, I think.

Copy link
Contributor

Choose a reason for hiding this comment

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

(And yes, it's true that you need to hard-code the color into the svg, so that it is replaced in MIconEnginePrivate::loadDataForModeAndState.)

(I'm looking forward to MuseScore4's icons system, which is a lot less complicated...)

@jeetee
Copy link
Contributor Author

jeetee commented Jan 29, 2021

@cbjeukendrup will PR a better suited fix.

@jeetee jeetee closed this Jan 29, 2021
@jeetee jeetee deleted the close-icon-MIDI-import-panel branch January 29, 2021 13:41
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.

2 participants