MediaController is refactored to:
- make the code easier to read and therefore more maintainable
- stop dumping errors like there's no tomorrow into the console
Details
Details
- Reviewers
- None
- Group Reviewers
Plasma
Use, ensure no regressions are found.
Diff Detail
Diff Detail
- Repository
- R120 Plasma Workspace
- Branch
- arcpatch-D29395
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 27031 Build 27049: arc lint + arc unit
Comment Actions
Now the inner image has a greater top margin than bottom margin. Also, could the artist and album be allowed to become two-line strings before eliding, maybe?
Comment Actions
Now the inner image has a greater top margin than bottom margin. Also, could the artist and album be allowed to become two-line strings before eliding, maybe?
... what? This patch doesn't (or shouldn't) result in visual changes.
I don't notice any visual changes on my system.
Comment Actions
Now the inner image has a greater top margin than bottom margin. Also, could the artist and album be allowed to become two-line strings before eliding, maybe?
There seems to be lots of wasted space whereas the text is truncated ?
Comment Actions
Generally +1
Nice idea with this Media singleton
| applets/mediacontroller/contents/ui/ExpandedRepresentation.qml | ||
|---|---|---|
| 65 ↗ | (On Diff #81831) | Coding style, space |
| 99 ↗ | (On Diff #81831) | Unrelated cleanup |
| applets/mediacontroller/contents/ui/Media.qml | ||
| 6 ↗ | (On Diff #81831) | Make this a QtObject |
| 87 ↗ | (On Diff #81831) | Just pass the action string through, since all we do below is map the number back to a string |
| 141 ↗ | (On Diff #81831) | or map() and then unshift the multiplexer :p |
| 187 ↗ | (On Diff #81831) | Remove debug prints |
| 197 ↗ | (On Diff #81831) | Turn this into a property with an if when this is no longer an Item |
| applets/mediacontroller/contents/ui/main.qml | ||
| 155 ↗ | (On Diff #81831) | You can just set state: Media.state rather than when on every state |
| 159 ↗ | (On Diff #81831) | We don't set album art on the icon anymore, cf D28917 |
| 243 | You can't remove these, they are wired up to the plasmoid.setAction calls above. (Yes, I'd like to have an API to pass a JS callback :p but this is how it works right now) | |
| applets/mediacontroller/contents/ui/ExpandedRepresentation.qml | ||
|---|---|---|
| 99 ↗ | (On Diff #81831) | a cleanup is unrelated in a cleanup patch? |
| applets/mediacontroller/contents/ui/Media.qml | ||
|---|---|---|
| 6 ↗ | (On Diff #81831) | Seems to cause a segfault with the data source. |
Comment Actions
Any chance to retake this and move it to invent?
This is really useful for an extension of the media controller I'm mantaining, and will help to eventually upsream it (al least partially)
I have a branch based on this MR with some fixes to it, and collaboration in gitlab seems easier