Remove obsolete AIM, IRC, ICQ fields from broadcasting preferences closes #12432 - #16092
Remove obsolete AIM, IRC, ICQ fields from broadcasting preferences closes #12432#16092prayag-kachhia wants to merge 6 commits into
Conversation
|
Welcome at Mixxx! |
|
Not sure if this issue was suggesting to remove these metadata fields - I certainly know some community still exclusively relying on IRC for comms. |
Done |
I took some help from AI - AIM was shut down in 2017. ICQ officially shut down June 2024. Neither service exists anymore — users cannot meaningfully fill these fields. Album art / stream logo are the actually-used modern equivalents of these contact fields — SAM Broadcaster exposes them and modern listeners expect them |
|
Thanks for the info. |
|
IIUC the 'icq', 'aim' etc. are just arbitrary char* (a-z0-9) metadata fields we create, and maybe there are streaming hosts that look for any of these fields, maybe not. Meaning: if IRC is still alive let's keep it 🤷♂️ My reason for filing #12432 back then was simply that Mixxx itself looks kind of outdated if it provides masks for ancient technology ; ) |
|
That's fair. I think as long as we could add some backward compatibility, I'm happy to remove this by default! Looking at how it gets consumed, it looks like these just comes out as Key/Value pair, so maybe we could allow settings an arbitrary map? I am thinking some table view where you can set arbitrary pair like Would you like to try and suggest an interface revamp @prayag-kachhia ? |
Exactly, and I don't find "title" or "artist" there, so these are also just established identifiers?
That would be a list view? |
I think they are if this doc is accurate, so is
Yeah that sounds even nicer! |
yep, I imagine it be to a compact GUI and probably nice UX -- but I'm not keen on the c++ hazzle this implies 😆 |
|
@acolombier @ronso0 — Thank you both for the detailed feedback! I've done some research on how libshout metadata actually works and what's visible to listeners. Key findings:
I'm happy to implement the dynamic table approach ronso0 suggested. Tell me if I am understanding it correctly, there would be an 'Add', which would allow the user to select the type of metadata they want to send (instagram, discord, twitch etc). Happy to convert this to Draft and push a prototype UI soon.
I see the complexity that it implies here. Let me give it a shot, I'll post here if I face any challenges. Just to confirm, with regards to the existing presets, we would only keep IRC, and scrap the IQC and AIM fields. |
Yes, Ithink that's consensus now. |
7d05414 to
7cdb233
Compare
…rences These messenger services are not standard. Removed the UI fields, backend profile storage, and sidechain transmission code entirely. Fixes mixxxdj#12432
7cdb233 to
c32cdc0
Compare
|
Based on the feedback in the PR review, rather than just restoring IRC as a static field, this implementation replaces the legacy fields with a fully dynamic "Social & Contact Links" table. This allows streamers to add an arbitrary number of custom metadata key-value pairs (like Instagram, Discord, Spotify, or any custom platform) that get passed to the streaming server. Changes included:
Backwards Compatibility / Migration: Closes: #12432 PS.: AI was used to write and refine this, as I didn't want to write in markdown myself. I hope it is acceptable |
|
@ronso0 @acolombier Could you please review this and give me any feedback. I am a bit eager to hear on this |
|
I think this is aiming in the right direction. Some element of feedback:
One to be confirmed by @ronso0 - shall we move all metadata in this table? The challenge is that some like |
Well, it is custom. You can type in anything you think appropriate. I thought commonly used socials in a dropdown would make it more intuitive. I will still change if you say so. |
|
Thank you, looks good already. Some feedback/opinion the UI:
|
I have made the changes as suggested by @acolombier and @ronso0. Please review and let me know if it is meeting expectations. |
|
UI looks good, but why is the key field still a QComboBox? |
|
Wondering if we could use a QComboBox to suggest the standard metadata (public, stream name, description, website, ...)? |
|
I have changed and removed the QComboBox. I think this is ready to merge |
We could discuss this in a new issue, but if we think from first principles, why do we need to change it? Is it improving user experience? Please let me know if this sounds rude. I am here to learn. |
| } | ||
|
|
||
| void DlgPrefBroadcast::addSocialLinkRow(const QString& key, const QString& value) { | ||
| QWidget* rowWidget = new QWidget(this); |
There was a problem hiding this comment.
we use the 'p' prefix for pointer, see https://github.com/mixxxdj/mixxx/wiki/Coding-Guidelines#c-style-guide
| rowLayout->setContentsMargins(0, 0, 0, 0); | ||
|
|
||
| // Column 0: QComboBox (editable, no default items) | ||
| QComboBox* keyCombo = new QComboBox(rowWidget); |
| keyCombo->setEditable(true); | ||
| // As per review feedback, no default social platforms | ||
|
|
||
| if (!key.isEmpty()) { |
There was a problem hiding this comment.
Let's check if both key and value are not empty (trim first)
| } | ||
|
|
||
| void DlgPrefBroadcast::addSocialLinkRow(const QString& key, const QString& value) { | ||
| QWidget* rowWidget = new QWidget(this); |
There was a problem hiding this comment.
Don't use naked "new", see https://github.com/mixxxdj/mixxx/wiki/Coding-Guidelines#pointer-object-lifetimeownership
-> std::unique_ptr
| if (!key.isEmpty()) { | ||
| keyCombo->setCurrentText(key); | ||
| } | ||
| rowLayout->addWidget(keyCombo); |
There was a problem hiding this comment.
with std::unique_ptr and QLineEdit:
| rowLayout->addWidget(keyCombo); | |
| rowLayout->addWidget(pKeyEdit.release()); |
since layout takes ownership
| void DlgPrefBroadcast::btnAddSocialLinkClicked() { | ||
| addSocialLinkRow(); | ||
| } |
There was a problem hiding this comment.
would be good to align the function/var names with the UI labels and the object names in the ui file:
btnCustomMetadataAdd
btnCustomMetadataRemove
::btnCustomMetadataAddClicked()
etc.
| // Column 0: QComboBox (editable, no default items) | ||
| QComboBox* keyCombo = new QComboBox(rowWidget); | ||
| keyCombo->setEditable(true); | ||
| // As per review feedback, no default social platforms |
There was a problem hiding this comment.
obsolete with QLineEdit
| // As per review feedback, no default social platforms |
|
Thank you, mosty looks good and works as expected. Also, some UX ideas:
|
|
This PR is marked as stale because it has been open 90 days with no activity. |
|
@prayag-kachhia Do you have time to finish this? |
Yes, I will finish this before the next weekend. Thanks for the push. |


closes #12432