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

[WIP] Completely rebuild the app in QML and overhaul the codebase #36

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

LorenDB
Copy link

@LorenDB LorenDB commented Feb 28, 2024

This commit will shred every speck of Jekyll out of the welcome
app, replacing it with a nice QML interface. While I'm not going
for a pixel-perfect recreation of the app, I'm trying to get a
UI that looks mostly the same as the Jekyll version.

As part of the rewrite, I'm overhauling the codebase to use best
practices for modern Qt and open-source projects in general. This
includes adding a .clang-format file to keep the code looking nice.

This commit will shred every speck of Jekyll out of the welcome
app, replacing it with a nice QML interface. While I'm not going
for a pixel-perfect recreation of the app, I'm trying to get a
UI that looks mostly the same as the Jekyll version.

As part of the rewrite, I'm overhauling the codebase to use best
practices for modern Qt and open-source projects in general. This
includes adding a .clang-format file to keep the code looking nice.

Closes openSUSE#34
@@ -1,5 +1,7 @@
<RCC>
<qresource prefix="/">
<file>main.qml</file>
<file>ThemedButton.qml</file>
<file>qtquickcontrols2.conf</file>
Copy link
Member

Choose a reason for hiding this comment

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

Empty config file?

@@ -21,48 +33,63 @@ void Enabler::enableAutostart()
void Enabler::disableAutostart()
{
qDebug() << "disable autostart called";
QFile::copy(":/desktop-file/org.opensuse.opensuse_welcome.desktop", QDir::homePath() + "/.config/autostart/org.opensuse.opensuse_welcome.desktop");
QFile::copy(":/desktop-file/org.opensuse.opensuse_welcome.desktop",
QDir::homePath() + "/.config/autostart/org.opensuse.opensuse_welcome.desktop");
qDebug() << QDir::homePath() + "/.config/autostart/org.opensuse.opensuse_welcome.desktop";
}
bool Enabler::fileExists(QString path)
Copy link
Member

Choose a reason for hiding this comment

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

This can just be QFile::exists(path). The case that the path is a non-file shouldn't be relevant.

qDebug() << "there isn't a file to disable, so it's enabled";
return true;
}
}
void Enabler::toggle() {
void Enabler::toggle()
{
qDebug() << "checking if autostart is enabled";
Copy link
Member

Choose a reason for hiding this comment

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

This can now be just setAutostart(!autostartEnabled())

qtTranslator.load("qt_" + QLocale::system().name(),
QLibraryInfo::location(QLibraryInfo::TranslationsPath));
app.installTranslator(&qtTranslator);
if (qtTranslator.load(
Copy link
Member

Choose a reason for hiding this comment

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

Loading Qt's translations shouldn't be necessary

@@ -4,6 +4,6 @@ welcome = executable('opensuse-welcome',
welcome_sources,
processed_files,
include_directories : inc,
dependencies: qt5_dep,
dependencies: qt6_dep,
Copy link
Member

Choose a reason for hiding this comment

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

I'd just use qt_dep here

@lkocman
Copy link
Contributor

lkocman commented Apr 4, 2024

I see that the PR is still in a draft state is that true? I'm up for cleanup as it there was so much unsed code, last time I've tried to rework it for EOL notifier. Do we want to keep this particular welcome solution for the future. @hellcp @LorenDB ?

@hellcp
Copy link
Member

hellcp commented Apr 4, 2024

I think this is the step in the right direction, I have some plans on making it look nicer, but it's better to start with having a more solid base for the time being

@LorenDB
Copy link
Author

LorenDB commented Apr 6, 2024

@lkocman I have not put any work into this for a while as I've gotten busy with other things. I may be able to come back and work on it some more, though.

Also, how imperative is it that we keep the exact look and feel intact? If we decided to go for more of a themed look (i.e. use regular QML theming to fit with the system theme more) and/or combined the rewrite with a redesign, it would probably help reduce the effort since we would't have to port every little detail over.

@hellcp
Copy link
Member

hellcp commented Apr 6, 2024

I wouldn't worry about the exact theming, I might want to make the app look different in the future, but let's just get it rewritten first and we will think about how it will look after that

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.

4 participants