Skip to content

Conversation

@danirabbit
Copy link
Member

@danirabbit danirabbit commented Jul 8, 2021

This is all getting pretty complicated and #123 would make it even more complicated. This branch moves more of the image/icon logic to Notification so that Bubble can be pretty straightforward. It also makes fallbacks more straightforward to implement

  • Add a new property primary_icon. Notification server clients try to either give us a gicon from appinfo, an icon name, or a path to an icon. All of these can be handled by GIcon, so this lets us set a single property instead of having Bubble try to create an image by null checking several properties
  • Add a new property image. There are two ways clients could try to set an image: file path or (in the future) image data. This lets Bubble check for a single property to see if a MaskedImage has been created or not.
  • The dialog-information fallback is now handled in Notification only instead of both in Bubble and Notification
  • We no longer need to reset image_path to null since Bubble only checks for a MaskedImage
  • Masked image already scales pixbufs sent into it afaict, so we don't need to scale before passing the pixbuf in

@bluesabre
Copy link
Contributor

Tested here, everything seems to work as expected. Really nice cleanup of the code. :)

@danirabbit
Copy link
Member Author

@bluesabre if you're good with these changes, I would love a PR Approve :)

@danirabbit danirabbit requested a review from a team July 8, 2021 19:46
Copy link
Contributor

@bluesabre bluesabre left a comment

Choose a reason for hiding this comment

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

Approve, works for me!

@danirabbit danirabbit merged commit 04ace65 into master Jul 8, 2021
@danirabbit danirabbit deleted the image-rework branch July 8, 2021 21:51
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