Skip to content
This repository has been archived by the owner on Jun 21, 2024. It is now read-only.

QT6 and KF6 fixes #227

Closed
wants to merge 3 commits into from
Closed

QT6 and KF6 fixes #227

wants to merge 3 commits into from

Conversation

huantianad
Copy link
Contributor

Just fixing some of the issues I saw when trying to use the new kde framework packages when building. My code isn't the best nor I do know if this breaks QT5 support, but I'm putting this here in case you'd actually be interested in pursuing this.

@huantianad huantianad force-pushed the master branch 4 times, most recently from f8695ce to 3c8efa6 Compare March 18, 2024 01:00
@@ -50,8 +50,8 @@ void CentralWidget::setupWebView() {
notification->setText(notificationInfo->message());
notification->setPixmap(
QPixmap::fromImage(notificationInfo->icon()));
notification->setDefaultAction("View");
connect(notification, &KNotification::defaultActivated,
notification->addDefaultAction("View");
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This gives a warning since it's nodiscard, is there a way I can use the result of this method instead of &KNotification::defaultActivated? I don't know how to cpp 😛

target_link_libraries(discord-screenaudio KF5::GlobalAccel)
endif()

if(KF6Notifications_FOUND)
target_link_libraries(discord-screenaudio KF6::Notifications)
install(FILES assets/discord-screenaudio.notifyrc DESTINATION ${CMAKE_INSTALL_PREFIX}/share/knotifications6)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should we use KDEInstallDirs?

@maltejur
Copy link
Owner

Hey, thanks a lot. I have actually already started working on KF6 support in the kf6 branch, but I didn't have the time to finish working on that. I wasn't yet able to make my branch work on all combinations of Qt5/Qt6 and X11/Wayland. So when I find the time, I complete that and merge in potential changes you made.

@maltejur
Copy link
Owner

maltejur commented Apr 22, 2024

This has been added to the master branch a while ago.

@maltejur maltejur closed this Apr 22, 2024
@huantianad
Copy link
Contributor Author

Yep, thanks! The nixpkgs package is building fine with the new KF6 packages 🙏

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants