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

Skins: reload default.qss when (re)loading a skin #12219

Merged
merged 2 commits into from
Feb 7, 2024

Conversation

ronso0
Copy link
Member

@ronso0 ronso0 commented Oct 25, 2023

This makes the workflow of tweaking /res/skins/default.qss a bit... faster.
Previously, a restart rebuild was required because deafult.qss is imported via /res/mixxx.qrc

@ronso0 ronso0 added the skins label Oct 25, 2023
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

I have found an issue that is also present in:

QString style = QString::fromLocal8Bit(fileBytes);

Can you fix both?

QFile file(":/skins/default.qss");
if (file.open(QIODevice::ReadOnly)) {
QByteArray fileBytes = file.readAll();
QString style = QString::fromLocal8Bit(fileBytes);
Copy link
Member

Choose a reason for hiding this comment

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

I don't think that fromLocal8Bit is correct here, because default.qss has always the same encoding on all our targets.
Is it Latin1 or Utf8?

Copy link
Member Author

@ronso0 ronso0 Oct 25, 2023

Choose a reason for hiding this comment

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

ParserM3u uses Utf8 so I picked that 🤷
I moved the redundant parsing to an inline function. We could as well move it to a member function (incl. setStyleSheet()).

Copy link
Member Author

@ronso0 ronso0 Oct 26, 2023

Choose a reason for hiding this comment

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

an inline function

That was pointless (and couldn't be built on all OS), I moved it to a member function.

@ronso0 ronso0 force-pushed the skins-reload-default.qss branch from fc88702 to ff5bd28 Compare October 26, 2023 09:40
@ronso0 ronso0 force-pushed the skins-reload-default.qss branch from ff5bd28 to 74c708e Compare October 28, 2023 22:37
@ronso0 ronso0 marked this pull request as draft November 8, 2023 13:04
@ronso0 ronso0 force-pushed the skins-reload-default.qss branch from 74c708e to 2608a9d Compare November 8, 2023 13:22
@ronso0 ronso0 marked this pull request as ready for review November 8, 2023 13:22
@ronso0
Copy link
Member Author

ronso0 commented Nov 8, 2023

@daschuer I didn't test properly before. Now default.qss is truly reloaded without a rebuild or restart.
Please take a look.

Copy link

github-actions bot commented Feb 7, 2024

This PR is marked as stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the stale Stale issues that haven't been updated for a long time. label Feb 7, 2024
Copy link
Member

@daschuer daschuer left a comment

Choose a reason for hiding this comment

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

LGTM, thank you.

@daschuer daschuer merged commit 45c85d8 into mixxxdj:2.4 Feb 7, 2024
11 checks passed
@ronso0 ronso0 deleted the skins-reload-default.qss branch February 7, 2024 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
skins stale Stale issues that haven't been updated for a long time.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants