bouteillerAlan / archupdate

widget for kde plasma ~ count the pacman update available
GNU General Public License v3.0
37 stars 4 forks source link

QA branch for V4.1.1 #13

Closed bouteillerAlan closed 1 year ago

bouteillerAlan commented 1 year ago

Features :

Todo :

bouteillerAlan commented 1 year ago

Hey @luisbocanegra many thanks for this update, seem amazing :)

Do you think is possible to add an option for the systray ?

I don't know if it's a good idea to activate it by default, for example I like to position it where I want in my taskbar. Is it possible to add an "enable in systray" option or not at all? What do you think?

luisbocanegra commented 1 year ago

Do you think is possible to add an option for the systray ?

I don't know if it's a good idea to activate it by default, for example I like to position it where I want in my taskbar.

Thinking more about it I agree with you, better allow system tray but not enabled by default out and let the user to decide where to place the applet

Is it possible to add an "enable in systray" option or not at all? What do you think?

I don't think so, the only way would be with dbus activation.

Closest thing is Plasmoid.status to control whether it shows or not but only if is enabled by default (to e.g. make the widget appear only if there are updates and hide in the expanded area or even disappear from the tray otherwise).

bouteillerAlan commented 1 year ago

Need to test something for the height of the icon, add an option for the size maybe image image

bouteillerAlan commented 1 year ago

@luisbocanegra for the systray I can add a hint in the readme for the users that want to activate it ? Because it's available and activatable very easily image

bouteillerAlan commented 1 year ago

Bug when I add the applet to a vertical taskbar

file:///home/a2n/.local/share/plasma/plasmoids/a2n.archupdate.plasmoid/contents/ui/archupdate.qml:14:37: QML Compact: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row. Row will not function.

Seem to be that line (the commented one) in Compact.qml :

  Item {
    id: container
    height: row.itemSize
    width: row.width

    //anchors.centerIn: parent

    Components.PlasmoidIcon {

For some reason when I added the applet to the vertical taskbar the count is not update

The screenshot ![image](https://github.com/bouteillerAlan/archupdate/assets/30547569/3f3750cd-92cc-4d16-b355-296f198cea3d)

But the cmd is launch normally (the log is only for the applet in the vertical bar), no problems with the horizontal bar :thinking: image

luisbocanegra commented 1 year ago

Need to test something for the height of the icon, add an option for the size maybe image image

Maybe limit the badge width too

image

luisbocanegra commented 1 year ago

Bug when I add the applet to a vertical taskbar

file:///home/a2n/.local/share/plasma/plasmoids/a2n.archupdate.plasmoid/contents/ui/archupdate.qml:14:37: QML Compact: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row. Row will not function.

Seem to be that line (the commented one) in Compact.qml :

  Item {
    id: container
    height: row.itemSize
    width: row.width

    //anchors.centerIn: parent

    Components.PlasmoidIcon {

Yeah, to be honest I tend to use anchors when they are probably not required. Is Row needed for something you have planned? Item seems to work fine as the Compact.qml root element

luisbocanegra commented 1 year ago

For some reason when I added the applet to the vertical taskbar the count is not update The screenshot image

But the cmd is launch normally (the log is only for the applet in the vertical bar), no problems with the horizontal bar πŸ€” image

Here it is Vertical badge text needs to take generateResult() or make generateResult() change totalText so it only called once

    WorkspaceComponents.BadgeOverlay { // for the horizontal bar
      anchors {
        bottom: container.bottom
        right: container.right
      }
      text: generateResult()
      visible: !isPanelVertical
      icon: updateIcon
    }

    WorkspaceComponents.BadgeOverlay { // for the vertical bar
      anchors {
        verticalCenter: container.bottom
        right: container.right
      }
      text: totalText
      visible: isPanelVertical
      icon: updateIcon
    }
bouteillerAlan commented 1 year ago

Bug when I add the applet to a vertical taskbar

file:///home/a2n/.local/share/plasma/plasmoids/a2n.archupdate.plasmoid/contents/ui/archupdate.qml:14:37: QML Compact: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row. Row will not function.

Seem to be that line (the commented one) in Compact.qml :

  Item {
    id: container
    height: row.itemSize
    width: row.width

    //anchors.centerIn: parent

    Components.PlasmoidIcon {

Yeah, to be honest I tend to use anchors when they are probably not required. Is Row needed for something you have planned? Item seems to work fine as the Compact.qml root element

Yeah, that the tutorial code haha :laughing: I'm going to replace it with Item :)

bouteillerAlan commented 1 year ago

Bug when I add the applet to a vertical taskbar

file:///home/a2n/.local/share/plasma/plasmoids/a2n.archupdate.plasmoid/contents/ui/archupdate.qml:14:37: QML Compact: Cannot specify left, right, horizontalCenter, fill or centerIn anchors for items inside Row. Row will not function.

Seem to be that line (the commented one) in Compact.qml :

  Item {
    id: container
    height: row.itemSize
    width: row.width

    //anchors.centerIn: parent

    Components.PlasmoidIcon {

Yeah, to be honest I tend to use anchors when they are probably not required. Is Row needed for something you have planned? Item seems to work fine as the Compact.qml root element

Who okai my code is not good at all haha (I have made only half of the thing I wanted to do with the function) I have made a refacto.

luisbocanegra commented 1 year ago

Need to test something for the height of the icon, add an option for the size maybe image image

Maybe limit the badge width too

image

A bunch of ideas

Here a screenshot of badge scaled up/down manually and some padding on top, unfortunately breaks consistency and also is less readable. image

bouteillerAlan commented 1 year ago

I have made some edit, I think like that the height and the width is more consistent, for the moment I have done nothing for the label ^^

widget in taskbar : image

in the systray : image image

bouteillerAlan commented 1 year ago

Ok I have added the possibility to use a dot in place of the label. Because in any case if the text is too long in the label it will be unmanageable.

The color is editable via the setting, but I need your help for matching the theme color if the setting is set to a empty string :

    Rectangle {
      visible: dot
      height: container.height / 2.5
      width: height
      radius: height / 2
      color: dotColor === '' ? dotColor : dotColor // todo - add theme color if dotColor === ''
      anchors {
        right: container.right
        bottom: container.bottom
      }
    }

Any idea ? I'm testing with kirigami.theme (https://pointieststick.com/2023/06/14/call-to-action-easy-porting-opportunity-in-plasma/).

image image image

luisbocanegra commented 1 year ago

Ok I have added the possibility to use a dot in place of the label. Because in any case if the text is too long in the label it will be unmanageable.

The color is editable via the setting, but I need your help for matching the theme color if the setting is set to a empty string :

    Rectangle {
      visible: dot
      height: container.height / 2.5
      width: height
      radius: height / 2
      color: dotColor === '' ? dotColor : dotColor // todo - add theme color if dotColor === ''
      anchors {
        right: container.right
        bottom: container.bottom
      }
    }

Any idea ? I'm testing with kirigami.theme (pointieststick.com/2023/06/14/call-to-action-easy-porting-opportunity-in-plasma).

image image image

While Kirigami.Theme.textColor works (the ~svs~ svgs use textColor too). For plasma 5 is better to use PlasmaCore.Theme.textColor so it correctly follows either the plasma theme colors or current color scheme, and then switch to Krigami.Theme for plasma 6.

import org.kde.kirigami 2.20 as Kirigami
...
    Rectangle {
      visible: dot && isUpdateNeeded()
      height: container.height / 2.5
      width: height
      radius: height / 2
      color: dotColor === '' ? PlasmaCore.Theme.textColor : dotColor
      anchors {
        right: container.right
        bottom: container.bottom
      }
    }
luisbocanegra commented 1 year ago

Also changed the color text field to a color picker

image

luisbocanegra commented 1 year ago

Which one looks better? I like 3 for how compact it is.

  1. Screenshot_20231106_144512

  2. Screenshot_20231106_145139

  3. Screenshot_20231106_145334

bouteillerAlan commented 1 year ago
  • [ ] add the value in the tooltips

Which one looks better? I like 3 for how compact it is.

  1. Screenshot_20231106_144512
  2. Screenshot_20231106_145139
  3. Screenshot_20231106_145334
  • Other

Yes the 3 is very cool :) Thanks a lot for all the help !

luisbocanegra commented 1 year ago
  • [ ] add the value in the tooltips

Which one looks better? I like 3 for how compact it is.

  1. Screenshot_20231106_144512
  2. Screenshot_20231106_145139
  3. Screenshot_20231106_145334
  • Other

Yes the 3 is very cool :) Thanks a lot for all the help !

No problem!

There is a small issue with the tooltip, when the mouse enters the widget it displays the default one, and then moving more into the widget displays the custom one πŸ™ƒ.

Will push in a moment, maybe you can figure out why

bouteillerAlan commented 1 year ago
  • [ ] add the value in the tooltips

Which one looks better? I like 3 for how compact it is.

  1. Screenshot_20231106_144512
  2. Screenshot_20231106_145139
  3. Screenshot_20231106_145334
  • Other

Yes the 3 is very cool :) Thanks a lot for all the help !

No problem!

There is a small issue with the tooltip, when the mouse enters the widget it displays the default one, and then moving more into the widget displays the custom one πŸ™ƒ.

Will push in a moment, maybe you can figure out why

FYI the issue is only present in systray (horizontal or vertical). I'm looking into the clipboard code for any hints on how kde dev do that :) https://invent.kde.org/plasma/plasma-workspace/-/tree/Plasma/5.27/applets/clipboard?ref_type=heads

luisbocanegra commented 1 year ago

There is a small issue with the tooltip, when the mouse enters the widget it displays the default one, and then moving more into the widget displays the custom one πŸ™ƒ. Will push in a moment, maybe you can figure out why

FYI the issue is only present in systray (horizontal or vertical). I'm looking into the clipboard code for any hints on how kde dev do that :)

https://invent.kde.org/plasma/plasma-workspace/-/tree/Plasma/5.27/applets/clipboard?ref_type=heads

I think that one is more limited, I used Tooltip.qml here https://github.com/luisbocanegra/plasma-intel-gpu-monitor can you check if it has the same issue?

Could do myself but I'm currently on Plasma master (trying to port https://github.com/psifidotos/applet-window-title lol) and this thing breaks if I switch to Plasma 5 until next reboot

bouteillerAlan commented 1 year ago

There is a small issue with the tooltip, when the mouse enters the widget it displays the default one, and then moving more into the widget displays the custom one πŸ™ƒ. Will push in a moment, maybe you can figure out why

FYI the issue is only present in systray (horizontal or vertical). I'm looking into the clipboard code for any hints on how kde dev do that :)

https://invent.kde.org/plasma/plasma-workspace/-/tree/Plasma/5.27/applets/clipboard?ref_type=heads

I think that one is more limited, I used Tooltip.qml here https://github.com/luisbocanegra/plasma-intel-gpu-monitor can you check if it has the same issue?

Could do myself but I'm currently on Plasma master (trying to port https://github.com/psifidotos/applet-window-title lol) and this thing breaks if I switch to Plasma 5 until next reboot

The problem seem resolve with that, but the width is messed up : https://invent.kde.org/plasma/plasma-workspace/-/blob/Plasma/5.27/applets/digital-clock/package/contents/ui/main.qml?ref_type=heads#L82

Yes same with the v0.1.1, I think we have to use Plasmoid.toolTipItem

bouteillerAlan commented 1 year ago

@luisbocanegra I have updated the code in 94e5103 for test if you want, ~rest to pass the total value to the tooltips if everything is ok for you :)~ (done in e73e136)

luisbocanegra commented 1 year ago

@luisbocanegra I have updated the code in 94e5103 for test if you want, ~rest to pass the total value to the tooltips if everything is ok for you :)~ (done in e73e136)

Oh it worked? Interesting

bouteillerAlan commented 1 year ago

@luisbocanegra I have updated the code in 94e5103 for test if you want, ~rest to pass the total value to the tooltips if everything is ok for you :)~ (done in e73e136)

Oh it worked? Interesting

Why ? πŸ˜‚

luisbocanegra commented 1 year ago

@luisbocanegra I have updated the code in 94e5103 for test if you want, ~rest to pass the total value to the tooltips if everything is ok for you :)~ (done in e73e136)

Oh it worked? Interesting

Why ? πŸ˜‚

Looks weird but if it works it works πŸ˜†

bouteillerAlan commented 1 year ago

@luisbocanegra I have updated the code in 94e5103 for test if you want, ~rest to pass the total value to the tooltips if everything is ok for you :)~ (done in e73e136)

Oh it worked? Interesting

Why ? πŸ˜‚

Looks weird but if it works it works πŸ˜†

that a bug ? no it's a feature !

luisbocanegra commented 1 year ago

Just tested all the options and panel locations, everything seems to be working correctly.

bouteillerAlan commented 1 year ago

Just tested all the options and panel locations, everything seems to be working correctly.

Ok ! Me too, so let's go for a release I think :)

bouteillerAlan commented 1 year ago

Release done @luisbocanegra, again thanks you so much for the help 🍻

luisbocanegra commented 1 year ago

You're welcome!