partial removal of isRadio and duplicated code in TrackMetadataModel
Details
- Reviewers
ngraham jguidon astippich - Group Reviewers
Elisa - Commits
- R255:a1ca107d948f: fix issues related to radio and view details in playlist
could not spot regressions
all automatic tests are OK except elisaqmltests that does not run due to an issue with a missing so dependency on KIO
Diff Detail
- Repository
- R255 Elisa
- Branch
- fixViewDetailInPlayList
- Lint
No Linters Available - Unit
No Unit Test Coverage - Build Status
Buildable 15716 Build 15734: arc lint + arc unit
deleting a radio is not possible anymore, and upon opening the metadata view qml shows some warnings
I just have two questions about the behavior:
- Does it make sense to display the Delete button while we are creating a radio ?
- The Apply button is not disabled after the radio is saved, maybe the user is not sure if the data has been successfully saved? I tried to keep the same behavior as the System Settings for instance.
- do not show delete button when creating a new radio
- partially fix the radio modification case
- final fix for radio modification
- fix radio modification by querying all data of modified radio
fix issues reported by @jguidon
I may have two wrong test cases and some minor comments about the code :)
The Delete button does not reappear after a saving a new radio.
The Apply button has no action after these steps:
1- Create a new radio -> Apply 2- Keep the window opened, try to modify a field 3- it looks like the Apply button does not trigger saving any more, the radio data is not saved in the database.
src/qml/MediaTrackMetadataView.qml | ||
---|---|---|
33–37 | Is this variable used ? | |
202 | I guess we do not care if the data has been successfully saved in the database ? (maybe I think too much like a web application :) ) |
- use MediaTrackMetadataView.editableMetadata to choose delegates
- fix modification of a newly created radio
fix some issues reported by @jguidon
src/qml/MediaTrackMetadataView.qml | ||
---|---|---|
202 | I do not think that update of a radio can reasonably fail no matter which data is set. One nice addition could be a check for field length if it makes sense. At least for me, this is a subject for a latter patch. |
This issue is fixed but deletion of a newly created radio is not working.
Should I wait for it to be fixed before reviewing ?
Also, I locally made some work to add album image support in the radio view based on the work of this review. I will wait for this review to be landed before pushing a new one.
- fix radio deletion and warning messages
- do not show delete button when creating a new radio
- partially fix the radio modification case
- final fix for radio modification
- fix radio modification by querying all data of modified radio
- use MediaTrackMetadataView.editableMetadata to choose delegates
- fix modification of a newly created radio
- show delete button once the new radio has been created
- fix deletion of a newly created radio
- fix failling automatic tests
Should be good to land now.
Let me know.
Thanks @astippich for the ping on this