partial removal of isRadio and duplicated code in TrackMetadataModel
ngraham jguidon astippich
- Group Reviewers
- 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
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.
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.
Is this variable used ?
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 :) )
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