Add second line of text for KickerDash item descriptions
ClosedPublic

Authored by sharvey on May 25 2018, 9:33 PM.

Details

Summary

Adds a second line of text under icon/doc icons for more clarity.
Will stop at two lines; if text continues, it will be elided.

BUG: 362986

Test Plan
  • Install Application Dashboard, identify a one-line elided item
  • Apply patch and compile
  • Check that item text is now two lines but does not overlap next row

Diff Detail

Repository
R119 Plasma Desktop
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
sharvey created this revision.May 25 2018, 9:33 PM
Restricted Application added a project: Plasma. · View Herald TranscriptMay 25 2018, 9:33 PM
sharvey requested review of this revision.May 25 2018, 9:33 PM
sharvey added a subscriber: Plasma.May 25 2018, 9:35 PM
Restricted Application removed a subscriber: Plasma. · View Herald TranscriptMay 25 2018, 9:35 PM
sharvey edited the test plan for this revision. (Show Details)May 25 2018, 9:35 PM
sharvey added inline comments.
applets/kicker/package/contents/ui/ItemGridDelegate.qml
98

Note: will wrap at an arbitrary point if there's no natural break. Happens with long filenames under "Recent Documents" sometimes. Otherwise, wraps at word breaks.


Unfortunate line breaks in long file names. See line 98.

ngraham accepted this revision.May 26 2018, 2:58 AM

Yep, this is exactly how I'd have done it, too.

Since this fixes https://bugs.kde.org/show_bug.cgi?id=362986, let's add BUG: 362986 at the bottom of the Summary (if you do it with the web interface, don't forget to do arc amend to pull the changes down to your local copy).

applets/kicker/package/contents/ui/ItemGridDelegate.qml
98

Trailing space.

This revision is now accepted and ready to land.May 26 2018, 2:58 AM
sharvey edited the summary of this revision. (Show Details)May 26 2018, 3:12 AM

I forgot it was a bug. I thought it was an assignment. :-)

sharvey updated this revision to Diff 34895.May 26 2018, 3:16 AM
  • - Remove trailing whitespace
ngraham accepted this revision.May 26 2018, 3:17 AM
sharvey marked 2 inline comments as done.May 26 2018, 3:17 AM
ngraham edited reviewers, added: Plasma; removed: plasma-devel.May 26 2018, 3:18 AM
Restricted Application added a subscriber: plasma-devel. · View Herald TranscriptMay 26 2018, 3:18 AM
This revision was automatically updated to reflect the committed changes.

Hmm, I might have waited for @hein's review too... One thing I'm myself just now coming to understand is that it's often a good idea to wait for more than just one reviewer to offer their view.

Oh. I see. The Phab emails say "now ready to land", so I did what the machine told me to. But I clearly see your point.

Hopefully he will be okay with it. If not, I'm more than willing to roll it back and make any further changes.

hein added a comment.May 27 2018, 10:45 AM

I'm fine with this, but yes, in the future it's a good idea to wait for maintainer review, or e.g. one VDG and one "code monkey" review - we do a bad job communicating who "needs" to approve though so it's not really on you, and I'm OK with erring on the side of work proceeding. Worst case, things don't work out OK and get reverted (say, temporarily).

Message received and understood. Sorry for jumping the gun.