DlgTrackInfo: Use TrackRecord as model #3906
Conversation
Save all edits temporarily in a local TrackRecord and replace them in the Track object all at once on apply. This is required to keep the properties in sync and valid while editing. Validation should not be done by the dialog itself but by TrackRecord which serves as the model that contains the domain logic. This change becomes crucial when properties dependent on each other, e.g. when storing genres both as multiple, custom tags and redundantly formatted in a text property. Those properties need to be synchronized mutually with each other.
|
I suppose this conflicts a bit with #3924, right? Can we return a QFlags when replacing the trackrecord that reflects which values actually changed and then emit changed signals on the track? |
|
@Holzhaus Thanks for testing! Editing the genre field works differently with custom tags. I forgot to add the signal handler that is needed temporarily until custom tags are ready. I also found |
Unrelated. The flags would need to be passed internally to Track::emitMetadataChanged() within |
|
If I change the Genre, the key disappears:
|
| // Special case handling for the comment field that is not | ||
| // updated by the editingFinished signal. | ||
| m_trackRecord.refMetadata().refTrackInfo().setComment(txtComment->toPlainText()); |
There was a problem hiding this comment.
Why is it not updated by the editingFinished signal?
There was a problem hiding this comment.
This has not changed. Because QPlainTextEdit only has a textChanged() but no editingFinished() signal?
|
@Holzhaus I force pushed the last commit, which was not finished yet. The changes not only fix the key text synchronization bug but also remove redundant code and improve the data binding with |
With our current QML implementation we can keep using legacy widgets such as the DlgTrackInfo side by side. In the long-term: Yes. |
|
This was also a perfect example for marking a data type as |
Save all edits temporarily in a local TrackRecord and replace them in the
Track object all at once on apply. This is required to keep the properties
in sync and valid while editing. Validation should not be done by the
dialog itself but by TrackRecord which serves as the model that contains
the domain logic.
This change becomes crucial when properties dependent on each other,
e.g. when storing genres both as multiple, custom tags and redundantly
formatted in a text property. Those properties need to be synchronized
mutually with each other.
The control and data flow is still far from perfect but it is hopefully a first
step in the right direction.
Should also fix https://bugs.launchpad.net/mixxx/+bug/1929311/comments/18