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

wip: add applet module #139

Merged
merged 7 commits into from
Aug 18, 2023
Merged

wip: add applet module #139

merged 7 commits into from
Aug 18, 2023

Conversation

wash2
Copy link
Contributor

@wash2 wash2 commented Aug 8, 2023

This seems to address the issues from pop-os/cosmic-applets#110 but maybe a separate trait could be added which adds the style by default.

@wash2 wash2 requested review from mmstick and ids1024 August 8, 2023 22:27
src/app/applet/mod.rs Outdated Show resolved Hide resolved
Base automatically changed from theme-update to master August 14, 2023 16:31
@wash2 wash2 marked this pull request as ready for review August 14, 2023 16:31
cosmic-theme/Cargo.toml Outdated Show resolved Hide resolved
Cargo.toml Outdated Show resolved Hide resolved
@ids1024
Copy link
Member

ids1024 commented Aug 15, 2023

This actually wasn't compiling with the applet feature, so I added a commit that fixed two little things that were breaking it.

Using this in an applet, it seems like 20a5227 has changed the styling for the applet buttons and buttons within them so the hover state isn't (very) visibly different. Changing Button::Text => &cosmic.text_button back to Button::Text => &theme.current_container().component reverts to the previous styling. But I guess that's something we want to change in the default theme instead?

@ids1024
Copy link
Member

ids1024 commented Aug 15, 2023

Otherwise this is looking good. I wonder if style() could be handled automatically somehow when applet::run is used to avoid that (small) boilerplate. And then there's the need to create a CosmicAppletHelper while the code in cosmic::app::applet is already doing that, though that was already duplicated.

@wash2
Copy link
Contributor Author

wash2 commented Aug 15, 2023

And then there's the need to create a CosmicAppletHelper while the code in cosmic::app::applet is already doing that, though that was already duplicated.

I think applets can just use self.core.applet_helper now instead of creating an CosmicAppletHelper, or am I missing something?

@ids1024
Copy link
Member

ids1024 commented Aug 15, 2023

Ah right, it's a property of core. So yeah, that part is simple enough.

@ids1024
Copy link
Member

ids1024 commented Aug 17, 2023

It doesn't seem quite right with the last commit either. The power applet here is styled using this branch, while the other applets are old versions.

eDP-1

Presumably when the applet isn't focused we still want the background to match the panel or be transparent.

@wash2
Copy link
Contributor Author

wash2 commented Aug 18, 2023

It doesn't seem quite right with the last commit either. The power applet here is styled using this branch, while the other applets are old versions.

Ahh ok, I see what you mean. I'll just make it use a custom style then?

@mmstick
Copy link
Member

mmstick commented Aug 18, 2023

Perhaps the hover state visibility could be a property of the Appearance struct, or as an input parameter to our methods? I'd say do what's necessary at the moment. Our button widget refactor will have better control over styling.

@ids1024
Copy link
Member

ids1024 commented Aug 18, 2023

Looks like the appearance when not hovered is still like my last screenshot. With a somewhat darker circle for the button instead of matching the panel.

Previously this used a completely transparent background for the button when not hovered, and an opaque background when hovered. So I guess we need that to get the same behavior.

A over A = A for opaque colors, but for semi-transparent colors it gets the same color but with a higher alpha.

@wash2
Copy link
Contributor Author

wash2 commented Aug 18, 2023

Looks like the appearance when not hovered is still like my last screenshot. With a somewhat darker circle for the button instead of matching the panel.

Previously this used a completely transparent background for the button when not hovered, and an opaque background when hovered. So I guess we need that to get the same behavior.

A over A = A for opaque colors, but for semi-transparent colors it gets the same color but with a higher alpha.

Oh sorry I forgot to make the buttons text buttons again after making the fix. Backgrounds of text buttons should be completely transparent.

Copy link
Member

@ids1024 ids1024 left a comment

Choose a reason for hiding this comment

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

I added a commit to not have "applet" depend on "a11y" since that was causing a panic... hopefully we can get that working well later.

Seems to be working fairly well now.

@wash2 wash2 merged commit 2086a0e into master Aug 18, 2023
8 checks passed
@wash2 wash2 deleted the cosmic-applet branch August 18, 2023 20:47
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