Fenix: [Design] Download notification accent color

[hakkikaancaliskan]

As @Amejia481 decided in https://github.com/mozilla-mobile/fenix/issues/12226#issuecomment-661069684 we're gonna stick with default notification.
Next thing @brampitoyo needs to decide is should we use fenix's accent color or let android use its own accent color https://github.com/mozilla-mobile/fenix/issues/12226#issuecomment-660899262.

[Amejia481]

With default colour

Normal theme

---

Dark theme

---

With accent colour

Normal theme

---

Dark theme

[hakkikaancaliskan]

@Amejia481 what if we set color of that icons to our accent color?

[hakkikaancaliskan]

Also i saw from your screenshots, why not to add a "x mins left" indicator?

[Amejia481]

@Amejia481 what if we set color of that icons to our accent color?

In my opinion, I would prefer in the normal default icon colour for the normal theme, but I guess it's a matter of preference, but in dark theme the accent colour looks pretty cool, in the icon and buttons!

Normal theme

---

Dark theme

The twitter app is changing the default accent colour and it looks pretty nice

[Amejia481]

Also i saw from your screenshots, why not to add a "x mins left" indicator?

I think this one will be a separate feature request ticket :)

[hakkikaancaliskan]

Agree with you they're soooo cool. I'm also on your side :)

[ghost]

Hey guys, please don't leave me alone... 🥺

Please tell me what's going too (I can already see what's going on though, so actually no need tell 😅)

:)

Edit: Before you mark this comment as off-topic, I'll let everyone know that this is something I had suggested in the https://github.com/mozilla-mobile/fenix/issues/12226#issuecomment-660897899 so I think I deserve to know what's going on here, telling you just because you don't delete this comment. That's why this weird sounding comment. Thank you. :)

[hakkikaancaliskan]

@Amejia481 #12734 opened issue
I've take a look at chromium source but unable to find how they added :(
Also unable to find online.

[hakkikaancaliskan]

Hey guys, please don't leave me alone... 🥺

Please tell me what's going too (I can already see what's going on though, so actually no need tell 😅)

:)

Edit: Before you mark this comment as off-topic, I'll let everyone know that this is something I had suggested in the https://github.com/mozilla-mobile/fenix/issues/12226#issuecomment-660897899 so I think I deserve to know what's going on here, telling you just because you don't delete this comment. That's why this weird sounding comment. Thank you. :)

You can just subscribe this issue and you will get notification on all conversation :)

[brampitoyo]

@Amejia481 @hakkikaancaliskan As long as we get the same icon animation (which is important to indicate visually that “something is going on”), I think the accent colour looks great!

Just making sure that our values are correct:

Light theme:

• Accent: Violet 70 #592acb

Dark theme:

• Accent: Violet 40 #ab71ff

[hakkikaancaliskan]

@brampitoyo Currently light theme is correct but dark is set to #815BDD
And best part is i can't find where we set accent color in fenix lol

[Amejia481]

I think for themes Fenix is using attributes, the accent colour is represented using R.attr.accent this property will update accordingly to the theme that fenix has light or dark, you can find an example of using it here

[hakkikaancaliskan]

I think for themes Fenix is using attributes, the accent colour is represented using R.attr.accent this property will update accordingly to the theme that fenix has light or dark, you can find an example of using it here

I know that but i mean where do we send accent color to ac for download notification?

[Amejia481]

Oh, sorry 😅 . We are not actually doing that now as it was not needed it before. Now we will have to implement it.
For changing the style colour on the notification, we need to expose an styling object from AC that Fenix can pass to the AbstractFetchDownloadService similar to what we are doing on AddonsManagerAdapter#Style.
An example can be how we pass the store and the httpClient. Hope it helps

[hakkikaancaliskan]

Oh, sorry 😅 . We are not actually doing that now as it was not needed it before. Now we will have to implement it.
For changing the style colour on the notification, we need to expose an styling object from AC that Fenix can pass to the AbstractFetchDownloadService similar to what we are doing on AddonsManagerAdapter#Style.
An example can be how we pass the store and the httpClient. Hope it helps

I don't have time currently but someone may work on this.

[hakkikaancaliskan]

@Amejia481 I've did with data class. Could you please review them?
AC side: https://github.com/mozilla-mobile/android-components/pull/9300
Fenix side: https://github.com/mozilla-mobile/fenix/pull/17231

[Amejia481]

@Amejia481 I've did with data class. Could you please review them?
AC side: https://github.com/mozilla-mobile/android-components/pull/9300
Fenix side: https://github.com/mozilla-mobile/fenix/pull/17231

Hi @hakkikaancaliskan I'm on PTO spending some quality time with family and friends during the holidays 🙂. I know others team members that are available will review, be aware reviews could take a bit longer as many people are on PTO too.
Happy holidays!

[hakkikaancaliskan]

@Amejia481 I've did with data class. Could you please review them?
AC side: https://github.com/mozilla-mobile/android-components/pull/9300
Fenix side: https://github.com/mozilla-mobile/fenix/pull/17231

Hi @hakkikaancaliskan I'm on PTO spending some quality time with family and friends during the holidays 🙂. I know others team members that are available will review, be aware reviews could take a bit longer as many people are on PTO too.
Happy holidays!

Ohh of course, I was expecting to be reviewed after holiday already, happy new year to all team members :)

See you after holiday ☺️ have a good time