ubuntu / app-center

App Store for Ubuntu made with Flutter ๐Ÿงก ๐Ÿ’™
GNU General Public License v3.0
665 stars 117 forks source link

chore: Bump dependencies #1818

Closed spydon closed 1 month ago

d-loose commented 2 months ago

Thanks for working through all those dependencies!

Some things I notices while testing a snap build of this:

light dark
Screenshot from 2024-09-19 16-29-23 Screenshot from 2024-09-19 16-29-15

I feel like it looks a bit off in the light theme

Feichtmeier commented 2 months ago

Thanks for working through all those dependencies!

Some things I notices while testing a snap build of this:

  • accent colors work correctly on both 24.04 and 24.10 - great work @Feichtmeier!
  • the background color and border style of the cards has changed again

light dark Screenshot from 2024-09-19 16-29-23 Screenshot from 2024-09-19 16-29-15 I feel like it looks a bit off in the light theme

np, yw!

indeed too strong! https://github.com/ubuntu/yaru.dart/blob/d0cc369a08bb5a7273b44cbdd3f059dc208ddbe2/lib/src/widgets/yaru_banner.dart#L94 don't know how I came to this color but it should just be cardcolor, and we need to make sure that cardcolor has an appr. contrast against surface however for app center you prbly dont want this strong contrast (I'm not saying this is perfect I need to look into it again, long time no use of this widget) so there are two color properties, I think you can just take scaffold background color or surface for both, I really need to look into this widget again, I think most things are not really needed (anymore)

d-loose commented 2 months ago

@spydon: looks like you left a TODO note for this :) https://github.com/ubuntu/app-center/blob/4fcd62b75d7b457c536f1ba5fe05a452241ba430/packages/app_center/lib/widgets/app_card.dart#L80

d-loose commented 2 months ago

I'm just noticing that yaru doesn't catch any gsettings exceptions in case accent colors aren't supported

flutter: ERROR main: Error propagated to top-level
    GSettingsUnknownKeyException: Key accent-color not in GSettings schema org.gnome.desktop.interface
#0      GSettings._getSchemaEntry (package:gsettings/src/gsettings.dart:199:7)
#1      GSettings.get (package:gsettings/src/gsettings.dart:120:23)
<asynchronous suspension>
#2      GnomeSettings._updateValue.<anonymous closure> (package:yaru/src/theme_widgets/settings_service.dart:59:31)
<asynchronous suspension>
Feichtmeier commented 2 months ago

I don't know if this is enough though When I look into the example I think it's too strong there too If you want to wait I can push ๐Ÿซธ a change/release later otherwise I would suggest you just overwrite the colour with whatever you want

d-loose commented 2 months ago

I'd definitely prefer making such changes in yaru.dart so that we can use it everywhere and get consistent themeing without needing to maintain any application-local customizations.

spydon commented 2 months ago

I think the color that we used to override with before was good? So YaruBanner now uses cardColor by default, but cardColor was changed, was there any reason for that change @Feichtmeier, if you remember? :)

Screenshot from 2024-09-19 17-19-43

Feichtmeier commented 2 months ago

I think the color that we used to override with before was good? So YaruBanner now uses cardColor by default, but cardColor was changed, was there any reason for that change @Feichtmeier, if you remember? :)

Screenshot from 2024-09-19 17-19-43

sorry I dont remember :older_man: just gimme a hex and I will make it work now (for all yaru.dart) edit: two hex

spydon commented 2 months ago

@Feichtmeier aah, it's defined in relation to the surface color:

Color _cardColor(ColorScheme colorScheme) =>
    colorScheme.surface.scale(lightness: colorScheme.isLight ? -0.1 : 0.08);

And before it seems like cardColor was just the surface color directly, seems like it would make sense to have them different at least? Maybe we should pull in @anasereijo here.

Feichtmeier commented 2 months ago

@Feichtmeier aah, it's defined in relation to the surface color:

Color _cardColor(ColorScheme colorScheme) =>
    colorScheme.surface.scale(lightness: colorScheme.isLight ? -0.1 : 0.08);

And before it seems like cardColor was just the surface color directly, seems like it would make sense to have them different at least? Maybe we should pull in @anasereijo here.

yes, the problem is that cardColor needs to have a contrast against the surface maybe we just use a different color (the one you wish) for YaruBanner?

Feichtmeier commented 2 months ago

is this good?

image image

Feichtmeier commented 2 months ago

or rather this?

image image

spydon commented 2 months ago

is this good?

image image

I think @anasereijo said she'd have some time to check tomorrow, so tagging her again ๐Ÿ˜„

Feichtmeier commented 2 months ago

ok I push something that looks like what you have in app center now if this is okay

light: scaffold color image

dark: slight contrast image

anasereijo commented 2 months ago

ok I push something that looks like what you have in app center now if this is okay

light: scaffold color image

dark: slight contrast image

Thanks @Feichtmeier this is looking much better IMO. I wasn't sure about the grey background on app cards, so thanks for making it possible to have the white. I quite like the subtle contrast in the dark theme too ๐Ÿ‘ Thanks!

spydon commented 2 months ago

@anasereijo this is what it looks like with Yaru 5.2.1: Screenshot from 2024-09-20 18-01-12 Screenshot from 2024-09-20 18-01-24