Currently the applet uses the main line in tooltip to display all info. Instead it should show always the same first line like other widgets and use the second line to show info.
BUG: 411973
FIXED-IN: 5.19.0
Details
Details
The point of this patch is to make the tooltip more like the style used in the Audio Volume applet:
Other "after" images:
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- arcpatch-D24070
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 20650 Build 20668: arc lint + arc unit
There are a very large number of changes, so older changes are hidden. Show Older Changes
Comment Actions
Secondly if you wait long enough, the description will change, meaning the remaining time isn't calculated imediately after plug/un-plug which looks like a bug somewhere else. This change only adds a message that remaining time isn't available yet.
| applets/batterymonitor/package/contents/ui/batterymonitor.qml | ||
|---|---|---|
| 70 | This early return still hasn't been addressed. | |
| applets/batterymonitor/package/contents/ui/batterymonitor.qml | ||
|---|---|---|
| 54 | You still haven't addressed this. Don't just return when powermanagementDisabled, I would still want to see my battery state when this is the case? | |
| applets/batterymonitor/package/contents/ui/batterymonitor.qml | ||
|---|---|---|
| 54 | OK, would you like to see: | |
Comment Actions
The second line is everywhere now. I guessed it would make more sense. But if you dont't like it that way, I can remove it.
Comment Actions
You can just do something like
var parts = [];
if (powerManagementDisabled) {
parts.push(i18n("Powermanagement disabled"));
}
if (pmSource.data["AC Adapter"] && pmSource.data["AC Adapter"]["Plugged in"]) {
parts.push(blabla);
} else {
parts.push(blablabla);
}
parts.push(whatever other texts);
return parts.join("\n");Comment Actions
Yeah, I'll take over the review. Working through some other stuff first, will resume later today or tomorrow.
Comment Actions
@ngraham could you please look at this? I would say it's finished and there isn't much time left until the next freeze. I also don't expect you to do this during the holidays, so it would be nice to have it finished before them. I am sorry for being impatient.
Comment Actions
@ngraham Sorry to bother you again, but tomorrow is repo freeze and it would be unfortunate if this doesn't make it in. I mean it's finished and it works correctly, right? If needed we can still do small polishing during the Beta period, correct?
Comment Actions
Yeah sorry, I need to block out some time to build a mental model of all the possible combinations currently, and what this changes them to, to make sure nothing regresses. Don't worry, this can go in after the soft feature freeze since it was already in progress, as long as it gets in before the hard feature freeze/string freeze.
Comment Actions
Power management disabled message is not show anymore if there is no battery present. Is that intentional?
Comment Actions
Anyway, anytime you feel like this is finished you can land it. You don't have to wait for me.
Comment Actions
Fix FIXME and don't show "Calculating blah..." text in subtitle when exact time isn't available
Comment Actions
@broulik I'm leery of landing this for 5.18 since today is the last day for string changes. Maybe we can land it early in the 5.19 cycle?
Comment Actions
Do we really have to? That would delay this by additional 5 months., which is really unfortunate since work (a very slow work but still...) on this begun about 4 moths ago. I mean this patch works correctly right? And even if we find an issue during Beta, it can still be fixed. That's what Betas are for, anyway.
Comment Actions
Strings can't be changed during the beta.
There really isn't a rush for this, is there? It's a pretty minor thing. I want it too, but it's more important to get it right than to get it in earlier.
| applets/batterymonitor/package/contents/ui/batterymonitor.qml | ||
|---|---|---|
| 87 ↗ | (On Diff #71222) | Why this early return? |