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

iconproducer: Add percentage to generated icon #294

Merged
merged 9 commits into from
Jan 21, 2022
Merged

Conversation

palinek
Copy link
Contributor

@palinek palinek commented Jan 14, 2022

  • before
    screenshot screenshot screenshot
  • after
    screenshot screenshot screenshot

@lxqt/lxqt Note: I'm not a designer, feel free to enhance the graphics

@palinek
Copy link
Contributor Author

palinek commented Jan 14, 2022

ref. #231

@stefonarch
Copy link
Member

stefonarch commented Jan 14, 2022

There is something not working here at 100%, full charged:
schermata-01-14-09-52-01

100= show + sign?

More generally: it doubles the tooltip info, so an option to show it or not could be implemented.
What eventually users could miss is an estimated "remaining time", no idea how hard this is to calculate.

EDIT: should be easy to show as tooltip:upower -i /org/freedesktop/UPower/devices/battery_BAT0|grep time|awk {'print $4,$5'}

@stefonarch
Copy link
Member

stefonarch commented Jan 14, 2022

Found a second issue:

  1. right click > configure
  2. select "use system icon", works
  3. unselect it : it doesn't switch back immediately but only when updated again, up to some minute.

EDIT: Could well be that this was always so.

@palinek
Copy link
Contributor Author

palinek commented Jan 14, 2022

@stefonarch Fixed the (old)100% bug

@stefonarch
Copy link
Member

Nice, thanks, so it was never noticed without the numbers?

My 2 cents:

  • some smaller size, for the numbers, with a % sign?
  • the minus sign. I first thought it was a graphical glitch or imperfection, because it's spilt by the green circle. But both look out of place somehow, in the center they were perfect. Another way to inform about charging/discharging should be found IMHO. Colors?
  • Is it possible to have both (ore even more) icons, to choose from in the settings, keeping also the old one?
  • And as said before, in the tooltip showing the same doesn't make much sense, but showing remaining time would be liked by many, even if this info is often not really reliable - seeing: "3,5 hours power left" can be more useful than "89%."

@palinek
Copy link
Contributor Author

palinek commented Jan 14, 2022

it was never noticed without the numbers?

Yes. It was there (hidden) intentionally, because of usage of the svg-path-arc...

the minus sign.

I used the pre-existing graphics, but just moved it to the left/right-upper corner, because leaving them underneath the percentage number was awkward.

Is it possible to have both (ore even more) icons, to choose from in the settings, keeping also the old one?

I don't think it is worth the hassles and code complication.

but showing remaining time would be liked by many

Added to tooltip.

@elviosak
Copy link

Another way to inform about charging/discharging should be found IMHO. Colors?

  • i think using colors could be a bit confusing, i associate it with available charge rather than charging status, like having green circle with more than 2/3, yellow for more than 1/3 and red for less than 1/3.

  • Up/down arrows?

  • lightning bolt when charging, nothing when discharging, that's how it shows on my phone, and i think it's good.

@stefonarch
Copy link
Member

stefonarch commented Jan 15, 2022

  • lightning bolt when charging, nothing when discharging, that's how it shows on my phone, and i think it's good.

I think this is the most suitable, or colors for the +/i signs. But we shouldn't overload the icon, we already have eventually the "pause" emblem when idleness checks are paused. Some mockups, I'm not at all familiar with svg:

schermata-01-15-08-51

without signs:

schermata-01-15-09-35

Also replace 100% with just nothing, or the lightning bolt or another symbol?

@tsujan
Copy link
Member

tsujan commented Jan 15, 2022

I haven't tried it yet but I think the problem is that we can't include several details in an icon that can be small. IMO, the percent sign can be removed because it makes the text too small. @palinek's first design (in his screenshots) was better; only the plus and minus signs could be raised to the top layer, maybe with different colors, as @stefonarch suggested. There's no space for ⚡.

@stefonarch
Copy link
Member

For me without the % it looks too much like a street sign, can't help.

@tsujan
Copy link
Member

tsujan commented Jan 15, 2022

For me without the % it looks too much like a street sign

LOL!

Personally, I think we'd better not take complaints about showing the number too seriously; otherwise, we might ruin the current neat design. Custom command plugin exists.

@tsujan
Copy link
Member

tsujan commented Jan 15, 2022

Or we could change the design completely, so that there's room for the text:

battery

@stefonarch
Copy link
Member

Had the same idea, if we change than better a new one, maybe alongside with the old one which many users are familiar with and which has a particular nice design.

@tsujan
Copy link
Member

tsujan commented Jan 15, 2022

I agree that keeping the old icon and adding a new one with the text is the best approach. The old icon isn't designed to include text.

But doing it will need time and motivation. I, for one, don't understand the urgency of showing the percentage on the icon or beside it.

@stefonarch
Copy link
Member

Anyway, the now improved tooltip is nice and should be merged - provide info when asked for.

@palinek
Copy link
Contributor Author

palinek commented Jan 18, 2022

Added new design of icon and updated the config GUI to let user choose between those 3.
Now with this we could easily change the code to get the theme icons, the preexisting one (w/o change), the circle one with text percentage, the battery one.
screenshot

@tsujan
Copy link
Member

tsujan commented Jan 18, 2022

Added new design of icon

The left design is very nice. The right one makes the text unreadable, and its reddish color is too alarming.

I think the translucent white background isn't needed with this design — the icon is also more attractive without it — and the whole icon can be made opaque.

@palinek
Copy link
Contributor Author

palinek commented Jan 18, 2022

The left design is very nice. The right one makes the text unreadable, and its reddish color is too alarming.

Both are the same design :). It is using the same green gradient, which is used in the pre-existing circle icon. The gradient is overlapped by pure red color (when discharging) with changing opacity from 0.0 - 60% to 1.0 - 0%. Maybe we can decrease the 60% level when the red begins to come into play.

@tsujan
Copy link
Member

tsujan commented Jan 18, 2022

I got it after seeing the code. IMHO, red is too alarming and makes the text hard to read on thin panels. Yellow?

@palinek
Copy link
Contributor Author

palinek commented Jan 18, 2022

This is with 30% breakpoint:
screenshot
screenshot
screenshot
screenshot

@tsujan
Copy link
Member

tsujan commented Jan 18, 2022

This is with 30% breakpoint:

They seem OK.

I'd remove the translucent square background and make the icon opaque, but I guess it's a matter of taste.

@palinek
Copy link
Contributor Author

palinek commented Jan 18, 2022

I'd remove the translucent square background and make the icon opaque

Then:

  1. we probably should move the charging sign into the battery completely to avoid invisibleness on particular backgrounds
  2. on some (green) backgrounds the battery can become hardly visible

Am I too cautious?

@tsujan
Copy link
Member

tsujan commented Jan 18, 2022

Am I too cautious?

No, you think logically, while I ignore rare special cases. You're right.

Perhaps we'd better merge it as it is; later, we could think about aesthetic details.

@palinek
Copy link
Contributor Author

palinek commented Jan 18, 2022

Perhaps we'd better merge it as it is

OK... so I'll update the PR to include 4 icon types:

  • from theme
  • pre-existing circle
  • new circle+percentage
  • new battery

@tsujan
Copy link
Member

tsujan commented Jan 18, 2022

Would be wonderful.

@stefonarch
Copy link
Member

stefonarch commented Jan 18, 2022

Nice work! Here the flash emblem isn't shown when plugged in (changed only text from bolder to normal - bold looks to strong for my taste).
schermata-01-18-17-35

@stefonarch
Copy link
Member

stefonarch commented Jan 18, 2022

When full charged the + emblem in the enhanced circle is trasformed in a dot which is hard to understand.
schermata-01-18-18-12

While testing different sizes and colors ecc I stumbled upon

 battery
    present:             yes
    rechargeable:        yes
    state:               pending-charge

and the icon tells "empty (97%)" and no +sign is shown.
schermata-01-18-18-27-01

@palinek
Copy link
Contributor Author

palinek commented Jan 19, 2022

I'd remove the translucent square background and make the icon opaque

While I'm on it... added the 5th type (just the "opaque" modification of the "battery" one):
screenshot

Now I think this should be ready to ship.

@stefonarch
Copy link
Member

There are some small issues left IMHO:

  • the mentioned dot when full and plugged in in circle_enhanced
  • text "normal "and font-size "80" ? Actually it's quite strong compared with other icons that show numbers, specially in circle_enhanced.
  • Naming (because hard to translate): circle-enhanced → "circle with percentage" or "circle with %"? Discard "built in"? It's not really necessary.
  • Adding also "Battery without %" ?

@stefonarch
Copy link
Member

Slightly reduced text, +/i and circle (full charged and plugged in) symbols, dark red color:
schermata-01-20-20-23 schermata-01-20-20-23-01
schermata-01-20-21-26

@tsujan
Copy link
Member

tsujan commented Jan 20, 2022

Slightly reduced text, +/i and circle (full charged and plugged in) symbols, dark red color:

IMHO, if a sign needs to be red, it'll be the minus sign.

Apart from the look, if the PR works fine now, please merge it. I agree that the appearance needs improvement and I have a plan to make a PR.

@palinek
Copy link
Contributor Author

palinek commented Jan 21, 2022

if the PR works fine now, please merge it. I agree that the appearance needs improvement and I have a plan to make a PR.

In opening comment I said directly, that I'm no designer...
@stefonarch are you OK with merging this and the appearance will be fine-tuned in following PR (by you or by tsujan)?

@stefonarch
Copy link
Member

@stefonarch are you OK with merging this and the appearance will be fine-tuned in following PR (by you or by tsujan)?

I am, but also the naming should be reconsidered as I mentioned

@palinek palinek merged commit de1bc2a into master Jan 21, 2022
@palinek palinek deleted the icon-enhance branch January 21, 2022 09:08
@tsujan
Copy link
Member

tsujan commented Jan 21, 2022

In opening comment I said directly, that I'm no designer...

IMO, the design is OK. I'll make only minor modifications, with @stefonarch's help.

tsujan added a commit that referenced this pull request Jan 21, 2022
This PR is only about the appearance — after #294. It doesn't change the functionality.
tsujan added a commit that referenced this pull request Jan 22, 2022
This PR is only about the appearance — after #294. It doesn't change the functionality.
tsujan added a commit that referenced this pull request Jan 22, 2022
This PR is only about the appearance — after #294. It doesn't change the functionality.
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.

4 participants