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

Allow to disable an icon in the dock during updates #2240

Merged
merged 2 commits into from
Jul 1, 2024

Conversation

sergio-costas
Copy link
Collaborator

@sergio-costas sergio-costas commented Jun 26, 2024

For the new RAA we need to be able to mark an icon as "disabled" while it is being updated. The current Unity LauncherAPI (https://wiki.ubuntu.com/Unity/LauncherAPI) doesn't support this, so this patch adds an extra option for this.

@sergio-costas
Copy link
Collaborator Author

@3v1n0 @vanvugt Can you review this when you have some spare time, please?

For the new RAA we need to be able to mark an icon as "disabled"
while it is being updated. The current Unity LauncherAPI
(https://wiki.ubuntu.com/Unity/LauncherAPI) doesn't support
this, so this patch adds an extra option for this.

https://docs.google.com/document/d/1--DgBRl6AqNiyjW_luOjl1dDzsIZPlF0Ukc7INW9_XQ
appIconIndicators.js Outdated Show resolved Hide resolved
if (this.updating)
icon.set_opacity(128);
else
icon.set_opacity(255);
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm wondering if these would be better as set_opacity_override, so as to prevent losing any previous custom opacity value. But I won't block on that -- it would only hurt the dock itself if such a loss ever happens so we'll deal with that later.

@vanvugt vanvugt merged commit 55f8310 into micheleg:master Jul 1, 2024
1 check passed
Comment on lines +194 to +195
this.connect('notify::updating', () => {
const icon = this.icon._iconBin;
Copy link
Collaborator

Choose a reason for hiding this comment

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

For next time, I think it would have been nicer to use this.bind_property_full.

However, I feel this code should be better to be as part of AppIconIndicators.IndicatorBase, no?

Since the appIcon is only the logical representation of an icon, shouldn't touch its look.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Now that I see the code with more detail, I don't think so: this code is in DockAbstractAppIcon, from which DockAppIcon and DockLocationAppIcon. Maybe it can be moved to DockAppIcon, but surely it has to be there, because it is what manages the appearance of the icon itself. IndicatorBase only manages the things that are over the icon. In fact, IndicatorBase manages the "updating" notification, and relays it to the corresponding DockAbstractAppIcon, which, I think, is the right way.

@3v1n0 3v1n0 changed the title UDENG-3111: allow to disable an icon in the dock during updates Allow to disable an icon in the dock during updates Jul 2, 2024
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.

3 participants