kmaterka (Konrad Materka)
User

Projects

User does not belong to any projects.

Today

  • Clear sailing ahead.

Tomorrow

  • Clear sailing ahead.

Sunday

  • Clear sailing ahead.

User Details

User Since
Jul 26 2019, 6:30 PM (17 w, 3 h)
Availability
Available

Recent Activity

Mon, Nov 18

kmaterka committed R135:3c786666453b: Avoid side effects during menu initialization (authored by kmaterka).
Avoid side effects during menu initialization
Mon, Nov 18, 1:12 PM
kmaterka closed D25223: Avoid side effects during menu initialization.
Mon, Nov 18, 1:12 PM · Plasma

Sat, Nov 16

kmaterka updated the diff for D23413: [System Tray] Unified data model for System Tray items.

Small code review change

Sat, Nov 16, 10:17 AM · Plasma
kmaterka added inline comments to D23413: [System Tray] Unified data model for System Tray items.
Sat, Nov 16, 10:16 AM · Plasma

Fri, Nov 15

kmaterka updated the diff for D23413: [System Tray] Unified data model for System Tray items.

Fixed one mistake during rebase. I checked it, seems it is still working.

Fri, Nov 15, 8:56 PM · Plasma
kmaterka updated the diff for D23413: [System Tray] Unified data model for System Tray items.

Rebased, but I need to test it

Fri, Nov 15, 7:14 PM · Plasma

Thu, Nov 14

kmaterka added a comment to D25223: Avoid side effects during menu initialization.

My long-term goal is to get rid of the application side KStatusNotifierItem and amend the QSystemTrayIcon API

OK, I will think about this. This change is only to fix one regression.
@broulik Is it OK now? Or should I use std::optional (and require C++17)?

Thu, Nov 14, 12:11 PM · Plasma

Tue, Nov 12

kmaterka added a comment to D25223: Avoid side effects during menu initialization.

I had this. I abandoned because we ended up forking some special wayland stuff in our DBus menu, so would always want our implementation.

?https://codereview.qt-project.org/c/qt/qtbase/+/168458

Having an option to fallback to default/predefined theme would be great, but I guess KDE needs to have it's own SystemTray implementation anyway (?). Some cleanup is required, because currently:

  • in Plasma-integration (QPA theme):
    • there is a forked QDBusMenuBar from Qt
    • for tray icon it uses KStatusNotifierItem which has it's drawbacks (additional layer)
  • in KNotifications, KStatusNotifierItem uses libdbusmenu-qt (not forked). It is optional dependency, is there any reason for that in 2019?
  • in plasma-workspace StatusNotifierItemSource uses DBusMenuImporter forked from libdbusmenu-qt
  • Qt has it's own implementation of DBus menu exporter:
    • in genericunix qpa theme, unfortuantelly QDBusPlatformMenu is private
    • it uses undocumented "NewMenu" signal
    • and is buggy: QTBUG-79287, KDE 383202
  • libdbusmenu-qt was not actively updated/maintained for years, I see that David made few commits in 2015. Shouldn't DBusMenuImporter be upstreamed?
  • Freedesktop spec is still in draft phase
Tue, Nov 12, 4:41 PM · Plasma
kmaterka added a comment to D25223: Avoid side effects during menu initialization.

Off-topic idea: This QPA integration uses KStatusNotifierItem, which then translates it to DBus. Wouldn't it be better to talk to DBus directly? From the other side, this may be a duplication of work... Was the idea discussed?

Tue, Nov 12, 2:11 PM · Plasma
kmaterka updated the diff for D25223: Avoid side effects during menu initialization.

QVariant related changes suggested in comments

Tue, Nov 12, 1:58 PM · Plasma
kmaterka added a comment to D25223: Avoid side effects during menu initialization.

I will send fixes in 5 minutes

Tue, Nov 12, 1:56 PM · Plasma

Sun, Nov 10

kmaterka added inline comments to D25223: Avoid side effects during menu initialization.
Sun, Nov 10, 8:12 PM · Plasma
kmaterka updated the diff for D25223: Avoid side effects during menu initialization.

Use QVariant for tri-state boolean. This way atributes are set only when they were really changed in the past.

Sun, Nov 10, 8:10 PM · Plasma

Fri, Nov 8

kmaterka updated the summary of D25223: Avoid side effects during menu initialization.
Fri, Nov 8, 9:27 PM · Plasma
kmaterka added a comment to D25223: Avoid side effects during menu initialization.

The problem was with:

m_menu->setVisible(m_visible);

setters/getters should not have side effects or any logic, but this method has. Detached menu stayed as top level window and rendered itself.

Fri, Nov 8, 9:26 PM · Plasma
kmaterka added a comment to D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.

This causes menus (mostly submenus) to randomly show up when the SNI is updated, e.g. every time VLC changes a track I get its "speed (slower, normal, faster)" menu open:

I will fix that right away!

Fixed in D25223

Fri, Nov 8, 9:20 PM · Plasma
kmaterka updated the diff for D25223: Avoid side effects during menu initialization.

Final version, ready for review.

Fri, Nov 8, 9:18 PM · Plasma
kmaterka added a comment to D25223: Avoid side effects during menu initialization.

Too soon, it is still not finished.

Fri, Nov 8, 9:00 PM · Plasma
kmaterka requested review of D25223: Avoid side effects during menu initialization.
Fri, Nov 8, 8:57 PM · Plasma
kmaterka added a comment to D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.

This causes menus (mostly submenus) to randomly show up when the SNI is updated, e.g. every time VLC changes a track I get its "speed (slower, normal, faster)" menu open:


(note the thick shadow, this is multiple menus stacked ontop of each other)

I will fix that right away!

Fri, Nov 8, 4:33 PM · Plasma

Tue, Nov 5

kmaterka committed R119:76d9233bd932: [Touchpad applet] Close query dialog on focus loss (authored by kmaterka).
[Touchpad applet] Close query dialog on focus loss
Tue, Nov 5, 4:46 PM
kmaterka closed D25158: [Touchpad applet] Close query dialog on focus loss.
Tue, Nov 5, 4:46 PM · Plasma
kmaterka added reviewers for D25158: [Touchpad applet] Close query dialog on focus loss: Plasma: Workspaces, Plasma, davidedmundson.
Tue, Nov 5, 2:46 PM · Plasma
kmaterka requested review of D25158: [Touchpad applet] Close query dialog on focus loss.
Tue, Nov 5, 2:45 PM · Plasma
kmaterka committed R242:96b7fc21f5b5: Add hideOnWindowDeactivate to PlasmaComponents.Dialog (authored by kmaterka).
Add hideOnWindowDeactivate to PlasmaComponents.Dialog
Tue, Nov 5, 1:56 PM
kmaterka closed D24825: Add hideOnWindowDeactivate to PlasmaComponents.Dialog.
Tue, Nov 5, 1:56 PM · Frameworks
kmaterka committed R120:2d7b6c2324c3: [SystemTray] Support for AttentionIcon (authored by kmaterka).
[SystemTray] Support for AttentionIcon
Tue, Nov 5, 1:55 PM
kmaterka closed D24865: [SystemTray] Support for AttentionIcon.
Tue, Nov 5, 1:55 PM · Plasma
kmaterka added a comment to D24865: [SystemTray] Support for AttentionIcon.

I will do more tests. The initialization

Tue, Nov 5, 1:54 PM · Plasma
kmaterka updated the diff for D24865: [SystemTray] Support for AttentionIcon.

Simply the check, thanks for a tip!

Tue, Nov 5, 1:44 PM · Plasma
kmaterka added a comment to D24865: [SystemTray] Support for AttentionIcon.

Alternative below. IMHO it might be cleaner.

Otherwise, this is fine.

Tue, Nov 5, 12:53 PM · Plasma
kmaterka added a comment to D24825: Add hideOnWindowDeactivate to PlasmaComponents.Dialog.

Should I proceed? This component has serious issues anyway, maybe there is no point in fixing this?
For example:
There is Loader that load always the same QML because... "if (true || ....)" - line 241. Even if this is obsolete, cleanup would be good idea.

Tue, Nov 5, 12:46 PM · Frameworks
kmaterka committed R363:56144c185b07: Add "authenticate" action (authored by kmaterka).
Add "authenticate" action
Tue, Nov 5, 12:09 PM
kmaterka closed D25155: Add "authenticate" action.
Tue, Nov 5, 12:09 PM
kmaterka updated the diff for D25155: Add "authenticate" action.

Minor typo

Tue, Nov 5, 12:09 PM
kmaterka added a task to D25155: Add "authenticate" action: T11977: [Print Manager] Add "authenticate" action and notification.
Tue, Nov 5, 11:54 AM
kmaterka added a revision to T11977: [Print Manager] Add "authenticate" action and notification: D25155: Add "authenticate" action.
Tue, Nov 5, 11:54 AM
kmaterka committed R135:bc1c85144adb: [KDEPlatformSystemTrayIcon] Recreate deleted menu (authored by kmaterka).
[KDEPlatformSystemTrayIcon] Recreate deleted menu
Tue, Nov 5, 11:43 AM
kmaterka closed D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.
Tue, Nov 5, 11:43 AM · Plasma
kmaterka added reviewers for D25155: Add "authenticate" action: ndavis, dantti, ngraham, broulik.
Tue, Nov 5, 10:31 AM
kmaterka requested review of D25155: Add "authenticate" action.
Tue, Nov 5, 10:27 AM
kmaterka triaged T11977: [Print Manager] Add "authenticate" action and notification as Low priority.
Tue, Nov 5, 10:02 AM

Mon, Nov 4

kmaterka added a comment to D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.

Any chance to get this reviewed? :)

Mon, Nov 4, 4:47 PM · Plasma
kmaterka added a reviewer for D24865: [SystemTray] Support for AttentionIcon: davidedmundson.
Mon, Nov 4, 4:45 PM · Plasma

Tue, Oct 29

kmaterka added inline comments to D24865: [SystemTray] Support for AttentionIcon.
Tue, Oct 29, 10:16 AM · Plasma

Mon, Oct 28

kmaterka added a comment to D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.

This is a proper solution to BUG 365105. Can someone review it? Is everything OK with this?

Mon, Oct 28, 10:12 AM · Plasma

Fri, Oct 25

kmaterka committed R120:82cfabd0a5c8: [DigitalClock] Fix layout and QML warnings (authored by kmaterka).
[DigitalClock] Fix layout and QML warnings
Fri, Oct 25, 1:36 PM
kmaterka closed D24853: [DigitalClock] Fix layout and QML warnings.
Fri, Oct 25, 1:36 PM · Plasma
kmaterka updated the diff for D24853: [DigitalClock] Fix layout and QML warnings.

Remove hardcoded margins, these are not needed anymore.

Fri, Oct 25, 1:08 PM · Plasma
kmaterka added inline comments to D24853: [DigitalClock] Fix layout and QML warnings.
Fri, Oct 25, 1:03 PM · Plasma

Thu, Oct 24

kmaterka added a comment to D24853: [DigitalClock] Fix layout and QML warnings.

It looks like there are some review changes required in D24798. Maybe I can commit my changes first? D24798 needs changes anyway and it would be good to split QCC1 -> QCC2 migration from layout fix.

Thu, Oct 24, 9:42 AM · Plasma

Oct 23 2019

kmaterka updated the diff for D24865: [SystemTray] Support for AttentionIcon.

Get rid of ugly and ridiculous nullness check.

Oct 23 2019, 11:49 AM · Plasma

Oct 22 2019

kmaterka requested review of D24865: [SystemTray] Support for AttentionIcon.

I added ugly way of checking if passed QVariant(QIcon) is null. Should I create native C++ method for that check or this can be done in QML, but in nicer way?

Oct 22 2019, 7:15 PM · Plasma
kmaterka added inline comments to D24865: [SystemTray] Support for AttentionIcon.
Oct 22 2019, 7:13 PM · Plasma
kmaterka updated the diff for D24865: [SystemTray] Support for AttentionIcon.

Fallback to Icon if there is no AttentionIcon.

Oct 22 2019, 7:10 PM · Plasma
kmaterka committed R885:2481a8eb6a22: Fix size of SpinBoxes (authored by kmaterka).
Fix size of SpinBoxes
Oct 22 2019, 5:04 PM
kmaterka closed D24861: Fix size of SpinBoxes.
Oct 22 2019, 5:04 PM · Plasma
kmaterka added inline comments to D24861: Fix size of SpinBoxes.
Oct 22 2019, 4:49 PM · Plasma
kmaterka updated the diff for D24861: Fix size of SpinBoxes.

Do not use hardcoded values

Oct 22 2019, 4:48 PM · Plasma
kmaterka added inline comments to D24798: Migrate QQC1 to QQC2.
Oct 22 2019, 2:26 PM · Plasma
kmaterka added a comment to D24853: [DigitalClock] Fix layout and QML warnings.

Some of this partially conflicts with D24798 FWIW.

Oct 22 2019, 1:58 PM · Plasma
kmaterka added a comment to D24865: [SystemTray] Support for AttentionIcon.

You might want to split that into a proper if statement otherwise it becomes somewhat hard to read. Something like

source: {
    if (taskIcon.status === PlasmaCore.Types.NeedsAttentionStatus && (AttentionIcon || AttentionIconName)) {
        return AttentionIcon || AttentionIconName;
    }
    return Icon || IconName;
}

The problem is that StatusNotifierItemSource always puts empty values for all elements. This is needed to initialize all roleNames (for QML delegate model etc). Anyway, it is always at least: "QVariant(QIcon, QIcon(null))".
In the situation when IconName was passed and not IconPixmap the check:

Oct 22 2019, 1:34 PM · Plasma
kmaterka added a comment to D24865: [SystemTray] Support for AttentionIcon.

I need to add one more check: when AttentionIcon is not set, it should fallback to Icon(Name)

Oct 22 2019, 1:02 PM · Plasma
kmaterka updated the diff for D24865: [SystemTray] Support for AttentionIcon.

Use enum for status check

Oct 22 2019, 12:59 PM · Plasma
kmaterka added a comment to D24865: [SystemTray] Support for AttentionIcon.

PlasmaCore.IconItem has a status property, so you probably need to explicitly check taskIcon.status then it should work with the proper enum

Oh! Now it looks obvious! I will switch to enum, as I intended, IMO with enum it looks cleaner (but longer)

Oct 22 2019, 12:57 PM · Plasma
kmaterka added a reviewer for D24865: [SystemTray] Support for AttentionIcon: Plasma: Workspaces.
Oct 22 2019, 12:53 PM · Plasma
kmaterka added a comment to D24767: [SystemTray] Support for AttentionIcon.

Doesn't this break the binding for source? My tray icons don't update anymore

Oct 22 2019, 12:53 PM · Plasma
kmaterka requested review of D24865: [SystemTray] Support for AttentionIcon.
Oct 22 2019, 12:51 PM · Plasma
kmaterka added a comment to D24767: [SystemTray] Support for AttentionIcon.

Doesn't this break the binding for source? My tray icons don't update anymore

Hmm, I will test that. It is possible :/ Definitely there is something wrong with PlasmaCore.IconItem.source binding. AttentionIcon was not shown when I did:

Oct 22 2019, 12:00 PM · Plasma
kmaterka added a reviewer for D24861: Fix size of SpinBoxes: Plasma.
Oct 22 2019, 11:49 AM · Plasma
kmaterka requested review of D24861: Fix size of SpinBoxes.
Oct 22 2019, 11:49 AM · Plasma
kmaterka updated the diff for D24853: [DigitalClock] Fix layout and QML warnings.

Review changes: Kirigami component, removed anchors

Oct 22 2019, 10:24 AM · Plasma
kmaterka added inline comments to D24853: [DigitalClock] Fix layout and QML warnings.
Oct 22 2019, 10:24 AM · Plasma
kmaterka added reviewers for D24853: [DigitalClock] Fix layout and QML warnings: Plasma, Plasma: Workspaces.
Oct 22 2019, 9:30 AM · Plasma
kmaterka requested review of D24853: [DigitalClock] Fix layout and QML warnings.
Oct 22 2019, 9:29 AM · Plasma
kmaterka added reviewers for D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu: Frameworks, broulik, nicolasfella.
Oct 22 2019, 6:34 AM · Plasma

Oct 21 2019

kmaterka retitled D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu from [KDEPlatformSystemTrayIcon] Recreate menu when deleted to [KDEPlatformSystemTrayIcon] Recreate deleted menu.
Oct 21 2019, 7:33 PM · Plasma
kmaterka added a comment to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

Moved to D24843

Oct 21 2019, 7:32 PM · Frameworks
kmaterka added a reviewer for D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu: Plasma.
Oct 21 2019, 7:32 PM · Plasma
kmaterka requested review of D24843: [KDEPlatformSystemTrayIcon] Recreate deleted menu.
Oct 21 2019, 7:31 PM · Plasma
kmaterka abandoned D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

This is a dead end, I will implement different solution in QPA (KDEPlatformSystemTrayIcon) plugin.

Oct 21 2019, 2:47 PM · Frameworks
kmaterka added a comment to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

First, it will not have memory leaks, we take ownership on parentless menu, on menu that has a parent, it will destroy it by parent-child cleanups.

Yes, eventually menu will be deleted. I'm thinking about the situation when someone has:

if (configuration-trayIconEnabled) {
   m_sni = new KStatusNotifierItem();
   m_sni->setContextMenu(new QMenu(**this**));
} else {
  delete m_sni;
}

and user is playing with this setting and changes it 100 times during one session. It will create 100 QMenu instances.

Oct 21 2019, 2:46 PM · Frameworks
kmaterka added a comment to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

That should be fine, in QPA we have a tray icon per sni, update menu should be on same object which will not be a problem, check it.

There are two objects in QPA that live independently:

  • KDEPlatformSystemTrayIcon (QPlatformSystemTrayIcon), with KSNI instance, KSNI and KDEPlatformSystemTrayIcon are destroyed on QSystemTrayIcon->hide() and new instance (with new KSNI) is created on QSystemTrayIcon->show()
  • SystemTrayMenu (QPlatformMenu) is not destroyed on QSystemTrayIcon->hide() and will be reused later on QSystemTrayIcon->show()
Oct 21 2019, 1:29 PM · Frameworks
kmaterka added a comment to D24825: Add hideOnWindowDeactivate to PlasmaComponents.Dialog.

This is a first step to solve 401016:
"Touchpad plasmoid [confirmation dialog] remains open when losing focus, I think that it is inconsistent since other elements of tray close themselves after switching away."

Oct 21 2019, 11:13 AM · Frameworks
kmaterka added reviewers for D24825: Add hideOnWindowDeactivate to PlasmaComponents.Dialog: Plasma, Frameworks.
Oct 21 2019, 11:09 AM · Frameworks
kmaterka requested review of D24825: Add hideOnWindowDeactivate to PlasmaComponents.Dialog.
Oct 21 2019, 11:07 AM · Frameworks

Oct 19 2019

kmaterka added inline comments to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.
Oct 19 2019, 4:14 PM · Frameworks
kmaterka added inline comments to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.
Oct 19 2019, 3:12 PM · Frameworks
kmaterka added a comment to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

I don't exactly like it, but I don't have anything better. +1

Me neither. Probably the best would be to use shared pointer, but it is not backward compatible.

Oct 19 2019, 1:38 PM · Frameworks
kmaterka updated the diff for D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

Wrap menu in QPointer

Oct 19 2019, 1:32 PM · Frameworks
kmaterka added inline comments to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.
Oct 19 2019, 10:18 AM · Frameworks
kmaterka committed R120:51964243ad40: [SystemTray] Support for AttentionIcon (authored by kmaterka).
[SystemTray] Support for AttentionIcon
Oct 19 2019, 8:14 AM
kmaterka closed D24767: [SystemTray] Support for AttentionIcon.
Oct 19 2019, 8:14 AM · Plasma

Oct 18 2019

kmaterka added a comment to D24767: [SystemTray] Support for AttentionIcon.

As part of:
https://www.freedesktop.org/wiki/Specifications/StatusNotifierItem/StatusNotifierItem/

Oct 18 2019, 10:22 PM · Plasma
kmaterka added reviewers for D24767: [SystemTray] Support for AttentionIcon: Plasma, Plasma: Workspaces.
Oct 18 2019, 10:10 PM · Plasma
kmaterka requested review of D24767: [SystemTray] Support for AttentionIcon.
Oct 18 2019, 10:08 PM · Plasma
kmaterka added inline comments to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.
Oct 18 2019, 9:19 PM · Frameworks
kmaterka added a comment to D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.

Continuation of D24667. I know this solution sucks, but I have no idea how this can be done better.

Oct 18 2019, 11:43 AM · Frameworks
kmaterka requested review of D24755: [KStatusNotifierItem] Optionaly, do not take menu ownership.
Oct 18 2019, 11:29 AM · Frameworks

Oct 17 2019

kmaterka committed R114:7ad4b59b7047: Correctly highlight the icon in the system tray (authored by kmaterka).
Correctly highlight the icon in the system tray
Oct 17 2019, 6:44 PM