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

Add battery widget to Shade skin #2283

Merged
merged 1 commit into from
Nov 12, 2019
Merged

Conversation

rrrapha
Copy link
Contributor

@rrrapha rrrapha commented Sep 15, 2019

preview

preview

I'm open for other suggestions..

@daschuer
Copy link
Member

It looks like this in my case:

grafik

You may consider to color only two and use the pink highlight color of the buttons.
The black for the full battery looks massive. Since it is a good normal state is should be more decent.

By the way, is the battery hidden, in case the device has no battery?

Looking at the ON AIR text, it looks wrong somehow. I think we should use the techno fonts from the other labels. We can also consider to use a question mark and a Mains Icon in this style.

It is quite hard though, here my attempt:
grafik

@rrrapha
Copy link
Contributor Author

rrrapha commented Sep 15, 2019

It looks like this in my case:

Are you running 2.2? This scaling issue should be fixed with 60c9a2e.

By the way, is the battery hidden, in case the device has no battery?

I think the <PixmapUnknown> is shown if there is no battery.

@daschuer
Copy link
Member

Ah yes, I will re-test with master.

Can we distinguish between no battery installed and unknown?
I think we should not bother desktop pc users with the question mark icon.

@daschuer
Copy link
Member

I think the charging battery should be always gray.

@rrrapha
Copy link
Contributor Author

rrrapha commented Sep 16, 2019

Can we distinguish between no battery installed and unknown?

We could distinguish between no battery detected and unknown.
IMO we should just hide the widget in both cases and never show the unknown icon.
Any opinions?

I think the charging battery should be always gray.

I won't change the colors for now, but it is easy to do using sed or similar.

@Be-ing
Copy link
Contributor

Be-ing commented Sep 16, 2019

We could distinguish between no battery detected and unknown.
IMO we should just hide the widget in both cases and never show the unknown icon.

I agree, and I don't think skins should be responsible for this. I think the WBattery widget code should take care of this.

@ronso0
Copy link
Member

ronso0 commented Sep 16, 2019

Can we distinguish between no battery installed and unknown?

We could distinguish between no battery detected and unknown.
IMO we should just hide the widget in both cases and never show the unknown icon.

When does 'unknown' occur?

@rrrapha
Copy link
Contributor Author

rrrapha commented Sep 16, 2019

When does 'unknown' occur?

The current implementation sets the state to Battery::UNKNOWN when no battery is
found or the status is unknown.
If I understand correcly, the status should not be UNKNOWN if a battery is found, but it can happen because of driver problems, broken batteries or similar.

@Be-ing
Copy link
Contributor

Be-ing commented Oct 22, 2019

Looks good to me. @daschuer ready for merge?

@rrrapha
Copy link
Contributor Author

rrrapha commented Oct 24, 2019

Here is another attempt using @daschuer's mains icon and different colors.
I am undecided .., I can update the pull request if you prefer this version.
preview
preview

@daschuer
Copy link
Member

The mains plug was one of may failed attempts. The rest looks good for me.
If no one has a better idea, I think we should go with the original flash or the pixeled "->".
I would be happy if you prepare such version.
Thank you :-)

@rrrapha
Copy link
Contributor Author

rrrapha commented Oct 24, 2019

Something like this?
preview
preview

@Holzhaus
Copy link
Member

Holzhaus commented Oct 24, 2019

@rrrapha Looks good IMO.

@Be-ing
Copy link
Contributor

Be-ing commented Nov 12, 2019

LGTM thank you @rrrapha.

@Be-ing Be-ing merged commit 4f13fb6 into mixxxdj:master Nov 12, 2019
@rrrapha rrrapha deleted the battery-shade branch February 25, 2020 15:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants