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

Improve battery icons in Deere skin #2250

Merged
merged 1 commit into from
Aug 27, 2019
Merged

Conversation

rrrapha
Copy link
Contributor

@rrrapha rrrapha commented Aug 26, 2019

This adds some color to the icons, like this:
preview

@daschuer
Copy link
Member

Thank you for the PR.
I think ic_battery_charging_8_48px.svg
Is missing. Even if the battery is 100% it can be charged. The flash signed can be used as "power supply working" state.

@rrrapha
Copy link
Contributor Author

rrrapha commented Aug 26, 2019

I think ic_battery_charging_8_48px.svg
Is missing. Even if the battery is 100% it can be charged. The flash signed can be used as "power supply working" state.

I agree that that ic_battery_charged_48px.svg should have the flash sign.
This icon is shown when the status is Battery::CHARGED.
If you're at 100% and pull the plug, the status changes to Battery::DISCHARGING and the nearest ic_battery_discharging* icon is shown.
Does this make sense?

preview

@daschuer
Copy link
Member

Does this make sense?

Yes, that's the point.

@ronso0
Copy link
Member

ronso0 commented Aug 27, 2019

Just noticed: I think the "battery state unknown" with a 'full' battery is misleading. Could it be a grey battery with a white ? ?

@rrrapha
Copy link
Contributor Author

rrrapha commented Aug 27, 2019

Just noticed: I think the "battery state unknown" with a 'full' battery is misleading. Could it be a grey battery with a white ? ?

Yes, that's true, I have force pushed the following version:
preview

@daschuer
Copy link
Member

Looks good now. Thank you.

@daschuer daschuer merged commit ee884cd into mixxxdj:master Aug 27, 2019
@rrrapha rrrapha deleted the deere-battery branch August 27, 2019 20:57
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