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

feat(tool,neon_framework): Update logo and use adaptive icons #2457

Merged
merged 1 commit into from
Sep 3, 2024

Conversation

provokateurin
Copy link
Member

@provokateurin provokateurin commented Sep 1, 2024

Closes #447

This was quite some work to get done, but I think it looks way better now:
image
image

It was not possible to split this into multiple commits as everything is coupled tightly.

@provokateurin provokateurin force-pushed the feat/neon_framework/update-logo-adaptive-icons branch from 2ea93b8 to 5ea55e0 Compare September 1, 2024 13:12
Copy link

codecov bot commented Sep 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 27.20%. Comparing base (91752b3) to head (65a8feb).
Report is 14 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2457      +/-   ##
==========================================
- Coverage   29.89%   27.20%   -2.69%     
==========================================
  Files         332      369      +37     
  Lines      124348   136655   +12307     
==========================================
+ Hits        37171    37179       +8     
- Misses      87177    99476   +12299     
Flag Coverage Δ *Carryforward flag
account_repository 99.64% <ø> (ø)
cookie_store 100.00% <ø> (ø) Carriedforward from 5c6b788
dashboard_app 96.05% <ø> (ø)
dynamite 31.08% <ø> (ø) Carriedforward from 5c6b788
dynamite_end_to_end_test 61.69% <ø> (-0.16%) ⬇️ Carriedforward from 5c6b788
dynamite_runtime 85.40% <ø> (ø) Carriedforward from 5c6b788
neon_dashboard 96.05% <ø> (ø) Carriedforward from 5c6b788
neon_framework 43.69% <ø> (+0.14%) ⬆️
neon_http_client 93.28% <ø> (ø) Carriedforward from 5c6b788
neon_notifications 100.00% <ø> (ø) Carriedforward from 5c6b788
neon_talk 99.45% <ø> (ø) Carriedforward from 5c6b788
nextcloud 23.26% <ø> (-2.65%) ⬇️ Carriedforward from 5c6b788
notifications_app 100.00% <ø> (ø)
sort_box 90.90% <ø> (ø) Carriedforward from 5c6b788
talk_app 99.09% <ø> (ø)

*This pull request uses carry forward flags. Click here to find out more.

see 42 files with indirect coverage changes

Copy link
Member

@Leptopoda Leptopoda left a comment

Choose a reason for hiding this comment

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

The diff LGTM

The only thing is that nextcloud apps use a gradient for the background. Can you also achieve that? Should be fairly simple.

@provokateurin
Copy link
Member Author

Ok I will give it a try, but it is probably a lot harder than you'd think.

@provokateurin provokateurin force-pushed the feat/neon_framework/update-logo-adaptive-icons branch from 5ea55e0 to f8fc86a Compare September 2, 2024 14:44
@provokateurin
Copy link
Member Author

Works for adaptive icons:
image
As for the non-adaptive icons it doesn't work because inkscape doesn't work with CSS gradient: Ignoring unrecognized CSS property: background-image.

@provokateurin
Copy link
Member Author

I just realized there are some mistakes...

@Leptopoda
Copy link
Member

As for the non-adaptive icons it doesn't work because inkscape doesn't work with CSS gradient: Ignoring unrecognized CSS property: background-image.

This shouldn't be a problem. Adaptive icons are supported since api 26 (8.0) while our min sdk is currently api 24 (7.0) so this shouldn't really matter.

Thanks for the quick changes; I'll do a review once you got it fixed and I have a bit of time.

@provokateurin provokateurin force-pushed the feat/neon_framework/update-logo-adaptive-icons branch from f8fc86a to 65a8feb Compare September 2, 2024 19:37
@provokateurin
Copy link
Member Author

This shouldn't be a problem. Adaptive icons are supported since api 26 (8.0) while our min sdk is currently api 24 (7.0) so this shouldn't really matter.

I agree. I think everywhere else we also show the icon in blue with transparent background, so there we don't need the gradient either.

Thanks for the quick changes; I'll do a review once you got it fixed and I have a bit of time.

Done

Copy link
Member

@Leptopoda Leptopoda left a comment

Choose a reason for hiding this comment

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

❤️

@provokateurin provokateurin merged commit da17de2 into main Sep 3, 2024
9 of 10 checks passed
@provokateurin provokateurin deleted the feat/neon_framework/update-logo-adaptive-icons branch September 3, 2024 05:09
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.

Update logo and use adaptive icons
2 participants