KristyF/PartyJukebox
1
0
Fork 0
Code Issues Pull requests Projects Releases Packages Wiki Activity Actions

Sockets allowing for live updates rather than complete rebuilds #7

Merged
kristy-fournier merged 20 commits from timerChanges into main 2026-02-11 09:39:36 -05:00
Conversation 1 Commits 20 Files changed 7 +309 -58
kristy-fournier commented 2026-02-11 09:10:15 -05:00 (Migrated from github.com)
Copy link
  • Queue can be added to and removed from without rebuild
  • Timer showing time remaining in song is live updated, and synced with the server every few seconds (i think 3)
  • Settings updated on the server are automatically sent to clients (through socket emit)
- Queue can be added to and removed from without rebuild - Timer showing time remaining in song is live updated, and synced with the server every few seconds (i think 3) - Settings updated on the server are automatically sent to clients (through socket emit)
copilot-pull-request-reviewer[bot] (Migrated from github.com) reviewed 2026-02-11 09:28:39 -05:00
copilot-pull-request-reviewer[bot] (Migrated from github.com) left a comment
Copy link

Pull request overview

Adds Socket.IO-based push updates so clients can stay in sync with the server (playlist changes, playback timer, and settings) without full page/playlist rebuilds.

Changes:

  • Introduces Flask-SocketIO on the server and emits events for queue/time/settings updates.
  • Updates the client to connect via Socket.IO and incrementally update playlist UI + timer state.
  • Minor UI/docs updates (controls centering tweak, README/Wishlist adjustments).

Reviewed changes

Copilot reviewed 6 out of 7 changed files in this pull request and generated 14 comments.

Show a summary per file
File Description
wishlist.md Updates wishlist items to reflect websocket-related progress.
requirements.txt Adds Flask-SocketIO and pins dotenv dependency.
readme.md Documentation updates for server structure and settings/features.
Server/webbyBits.py Adds Socket.IO server, emits songAdd/timeUpdate/skipSong/settingsChange, updates some API responses.
Client/styles.css Centers controls using CSS transform instead of JS margin calc.
Client/scripts.js Adds Socket.IO client + live playlist/time/settings update logic; updates fetch helper and control handlers.
Client/index.html Loads Socket.IO + js-sha256 from CDN; updates duplicate toggle label text.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

## Pull request overview Adds Socket.IO-based push updates so clients can stay in sync with the server (playlist changes, playback timer, and settings) without full page/playlist rebuilds. **Changes:** - Introduces Flask-SocketIO on the server and emits events for queue/time/settings updates. - Updates the client to connect via Socket.IO and incrementally update playlist UI + timer state. - Minor UI/docs updates (controls centering tweak, README/Wishlist adjustments). ### Reviewed changes Copilot reviewed 6 out of 7 changed files in this pull request and generated 14 comments. <details> <summary>Show a summary per file</summary> | File | Description | | ---- | ----------- | | wishlist.md | Updates wishlist items to reflect websocket-related progress. | | requirements.txt | Adds Flask-SocketIO and pins dotenv dependency. | | readme.md | Documentation updates for server structure and settings/features. | | Server/webbyBits.py | Adds Socket.IO server, emits `songAdd/timeUpdate/skipSong/settingsChange`, updates some API responses. | | Client/styles.css | Centers controls using CSS transform instead of JS margin calc. | | Client/scripts.js | Adds Socket.IO client + live playlist/time/settings update logic; updates fetch helper and control handlers. | | Client/index.html | Loads Socket.IO + js-sha256 from CDN; updates duplicate toggle label text. | </details> --- 💡 <a href="/kristy-fournier/PartyJukebox/new/main/.github/instructions?filename=*.instructions.md" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Add Copilot custom instructions</a> for smarter, more guided reviews. <a href="https://docs.github.com/en/copilot/customizing-copilot/adding-repository-custom-instructions-for-github-copilot" class="Link--inTextBlock" target="_blank" rel="noopener noreferrer">Learn how to get started</a>.
Client/index.html
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:37 -05:00
Copy link

The label’s for attribute doesn’t match the checkbox id (duplicateallowsettingcheck vs duplicateallowesettingcheckbox), so clicking the label won’t toggle the checkbox. Update the for to match the input id for proper accessibility and UX.

                    <input type="checkbox" title="duplicateallowcheck" id="duplicateallowesettingcheckbox"><label for="duplicateallowesettingcheckbox">Add duplicate songs</label><br>
The label’s `for` attribute doesn’t match the checkbox id (`duplicateallowsettingcheck` vs `duplicateallowesettingcheckbox`), so clicking the label won’t toggle the checkbox. Update the `for` to match the input id for proper accessibility and UX. ```suggestion <input type="checkbox" title="duplicateallowcheck" id="duplicateallowesettingcheckbox"><label for="duplicateallowesettingcheckbox">Add duplicate songs</label><br> ```
Client/scripts.js
@ -98,15 +111,22 @@ function getCookie(cname) {
// also someone who likes things not being dumb more than me would have separated the client and server buttons
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:36 -05:00
Copy link

else if (buttonType = "pm") assigns instead of comparing, so this branch will always execute and also mutates buttonType. Change this to a comparison (===) so only the intended button triggers the party mode toggle.

    } else if (buttonType === "pm") { //Partymode toggle (in settings)
`else if (buttonType = "pm")` assigns instead of comparing, so this branch will always execute and also mutates `buttonType`. Change this to a comparison (`===`) so only the intended button triggers the party mode toggle. ```suggestion } else if (buttonType === "pm") { //Partymode toggle (in settings) ```
Client/scripts.js
@ -98,15 +111,22 @@ function getCookie(cname) {
// also someone who likes things not being dumb more than me would have separated the client and server buttons
async function controlButton(buttonType) {
if (buttonType == "pp") { // Play-Pause button
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:38 -05:00
Copy link

In the non-OK response branch, throw new Error(data.error) makes the subsequent alertText(...) line unreachable, and also forces callers into the catch path even though you already have the server-provided error payload. Consider either removing the throw and returning the structured { ok/status/error } response, or moving the alert before throwing and ensuring callers handle a null return without crashing.
In the non-OK response branch, `throw new Error(data.error)` makes the subsequent `alertText(...)` line unreachable, and also forces callers into the catch path even though you already have the server-provided error payload. Consider either removing the throw and returning the structured `{ ok/status/error }` response, or moving the alert before throwing and ensuring callers handle a `null` return without crashing. ```suggestion ```
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:38 -05:00
Copy link

controlButton now dereferences result["data"]["playingState"] without checking for a null return from getFromServer (which happens on network errors). This will throw and break UI interaction when the server is unreachable; add a null/ok check before using result.data.

        if (result && result.ok && result["data"] && result["data"]["playingState"] !== undefined) {
            currentlyPlaying = result["data"]["playingState"];
        }
`controlButton` now dereferences `result["data"]["playingState"]` without checking for a `null` return from `getFromServer` (which happens on network errors). This will throw and break UI interaction when the server is unreachable; add a null/ok check before using `result.data`. ```suggestion if (result && result.ok && result["data"] && result["data"]["playingState"] !== undefined) { currentlyPlaying = result["data"]["playingState"]; } ```
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:39 -05:00
Copy link

Avoid automated semicolon insertion (93% of all statements in the enclosing function have an explicit semicolon).

Avoid automated semicolon insertion (93% of all statements in [the enclosing function](1) have an explicit semicolon).
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:38 -05:00
Copy link

In addToPlaylist, i is assigned without let/const, creating an implicit global. Also image.id = String(songObject[newItem.id]) + " image" will usually become "[object Object] image" and won’t be unique/meaningful. Declare i locally and base the image id on the filename (newItem.id) like the rest of the file does.

In `addToPlaylist`, `i` is assigned without `let/const`, creating an implicit global. Also `image.id = String(songObject[newItem.id]) + " image"` will usually become "[object Object] image" and won’t be unique/meaningful. Declare `i` locally and base the image id on the filename (`newItem.id`) like the rest of the file does.
Client/scripts.js
@ -441,2 +591,4 @@
let clickedBox = e.srcElement;
clickedBox.checked = !clickedBox.checked;
} else {
justChangedSetting = true;
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:39 -05:00
Copy link

skipInPlaylist() assumes playlistChildren[1] exists and calls displayElapsedPlaylistTime() even when the playlist DOM (and elapsed-time-display) may not exist (e.g., when not in playlist mode or the playlist was cleared). This can throw on socket-driven skipSong events. Add guards for playlist-mode visibility/child existence and update playlistSongLength/playlist state when advancing to the next song so the timer stays correct.

    // reset elapsed time for the (new) current song
    playlistElapsedSeconds = 0;

    // Keep the in-memory playlist and song length in sync when skipping
    if (Array.isArray(typeof playlist !== "undefined" ? playlist : null) && playlist.length > 0) {
        // Remove the song that was just skipped
        playlist.shift();
        if (playlist.length > 0) {
            const nextSong = playlist[0];
            let nextLength = -1;
            if (nextSong && typeof nextSong.length === "number") {
                nextLength = nextSong.length;
            } else if (nextSong && typeof nextSong.duration === "number") {
                nextLength = nextSong.duration;
            }
            playlistSongLength = nextLength;
        } else {
            // No more songs queued
            playlistSongLength = -1;
            if (playlistTimeTimer) {
                clearInterval(playlistTimeTimer);
            }
        }
    }

    const playlistElement = document.getElementById("playlist");
    if (!playlistElement) {
        // Playlist UI is not present; nothing more to update visually.
        const elapsedDisplay = document.getElementById("elapsed-time-display");
        if (typeof displayElapsedPlaylistTime === "function" && elapsedDisplay) {
            displayElapsedPlaylistTime(playlistElapsedSeconds, playlistSongLength);
        }
        return;
    }

    let playlistChildren = playlistElement.children;

    // Safely remove the first actual song entry, if it exists
    if (playlistChildren.length > 1 && playlistChildren[1].nodeName === "DIV") {
        playlistChildren[1].remove();
        playlistChildren = playlistElement.children;
    }

    if (playlistChildren.length === 0) {
        // No children at all; nothing further to do
    } else if (playlistChildren.length === 1) {
        // Only the alert/header element remains
        playlistChildren[0].innerText = "Nothing's Queued...";
    } else {
        // There is a new first song entry; carefully reassign IDs
        const firstEntry = playlistChildren[1];
        if (firstEntry && firstEntry.children && firstEntry.children.length > 1) {
            const textContainer = firstEntry.children[1];
            if (textContainer && textContainer.children && textContainer.children.length > 3) {
                let firstElementTextChildren = textContainer.children;
                // console.log(firstElementTextChildren);
                firstElementTextChildren[2].id = "elapsed-time-display";
                firstElementTextChildren[3].id = "playing-indicator-text";
                firstElementTextChildren[3].textContent = "Playing";
            }
        }
    }

    const elapsedDisplay = document.getElementById("elapsed-time-display");
    if (typeof displayElapsedPlaylistTime === "function" && elapsedDisplay) {
        displayElapsedPlaylistTime(playlistElapsedSeconds, playlistSongLength);
    }
`skipInPlaylist()` assumes `playlistChildren[1]` exists and calls `displayElapsedPlaylistTime()` even when the playlist DOM (and `elapsed-time-display`) may not exist (e.g., when not in playlist mode or the playlist was cleared). This can throw on socket-driven `skipSong` events. Add guards for playlist-mode visibility/child existence and update `playlistSongLength`/`playlist` state when advancing to the next song so the timer stays correct. ```suggestion // reset elapsed time for the (new) current song playlistElapsedSeconds = 0; // Keep the in-memory playlist and song length in sync when skipping if (Array.isArray(typeof playlist !== "undefined" ? playlist : null) && playlist.length > 0) { // Remove the song that was just skipped playlist.shift(); if (playlist.length > 0) { const nextSong = playlist[0]; let nextLength = -1; if (nextSong && typeof nextSong.length === "number") { nextLength = nextSong.length; } else if (nextSong && typeof nextSong.duration === "number") { nextLength = nextSong.duration; } playlistSongLength = nextLength; } else { // No more songs queued playlistSongLength = -1; if (playlistTimeTimer) { clearInterval(playlistTimeTimer); } } } const playlistElement = document.getElementById("playlist"); if (!playlistElement) { // Playlist UI is not present; nothing more to update visually. const elapsedDisplay = document.getElementById("elapsed-time-display"); if (typeof displayElapsedPlaylistTime === "function" && elapsedDisplay) { displayElapsedPlaylistTime(playlistElapsedSeconds, playlistSongLength); } return; } let playlistChildren = playlistElement.children; // Safely remove the first actual song entry, if it exists if (playlistChildren.length > 1 && playlistChildren[1].nodeName === "DIV") { playlistChildren[1].remove(); playlistChildren = playlistElement.children; } if (playlistChildren.length === 0) { // No children at all; nothing further to do } else if (playlistChildren.length === 1) { // Only the alert/header element remains playlistChildren[0].innerText = "Nothing's Queued..."; } else { // There is a new first song entry; carefully reassign IDs const firstEntry = playlistChildren[1]; if (firstEntry && firstEntry.children && firstEntry.children.length > 1) { const textContainer = firstEntry.children[1]; if (textContainer && textContainer.children && textContainer.children.length > 3) { let firstElementTextChildren = textContainer.children; // console.log(firstElementTextChildren); firstElementTextChildren[2].id = "elapsed-time-display"; firstElementTextChildren[3].id = "playing-indicator-text"; firstElementTextChildren[3].textContent = "Playing"; } } } const elapsedDisplay = document.getElementById("elapsed-time-display"); if (typeof displayElapsedPlaylistTime === "function" && elapsedDisplay) { displayElapsedPlaylistTime(playlistElapsedSeconds, playlistSongLength); } ```
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:39 -05:00
Copy link

Variable playlist is used like a local variable, but is missing a declaration.

Variable playlist is used like a local variable, but is missing a declaration.
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:35 -05:00
Copy link

Leftover debug logging (console.log(e)) in the volume slider handler will spam the console on every change. Remove this (or gate it behind a debug flag) before merging.

Leftover debug logging (`console.log(e)`) in the volume slider handler will spam the console on every change. Remove this (or gate it behind a debug flag) before merging.
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:37 -05:00
Copy link

The settingsChange socket handler still logs to console (console.log(data) / console.log("working")). These debug statements will be noisy for all clients; remove them once behavior is validated.

    if (justChangedSetting) {
The `settingsChange` socket handler still logs to console (`console.log(data)` / `console.log("working")`). These debug statements will be noisy for all clients; remove them once behavior is validated. ```suggestion if (justChangedSetting) { ```
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:36 -05:00
Copy link

Socket initialization uses an undeclared socket variable (implicit global) and the option name is misspelled (reconnectionAttemps). Declare with const/let and use the correct Socket.IO option name (reconnectionAttempts) so reconnection behavior works as intended.

let socket = io("http://"+ip,{
    reconnectionAttempts: 5,
Socket initialization uses an undeclared `socket` variable (implicit global) and the option name is misspelled (`reconnectionAttemps`). Declare with `const/let` and use the correct Socket.IO option name (`reconnectionAttempts`) so reconnection behavior works as intended. ```suggestion let socket = io("http://"+ip,{ reconnectionAttempts: 5, ```
Client/scripts.js
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:37 -05:00
Copy link

The Socket.IO event handlers (songAdd, skipSong) update the playlist DOM unconditionally. When the user is in search/settings mode, the code clears #playlist and these handlers can throw (e.g., skipInPlaylist indexing children). Gate these updates behind playlist-mode visibility or ensure the playlist DOM has the expected structure before mutating it.

The Socket.IO event handlers (`songAdd`, `skipSong`) update the playlist DOM unconditionally. When the user is in search/settings mode, the code clears `#playlist` and these handlers can throw (e.g., `skipInPlaylist` indexing children). Gate these updates behind `playlist-mode` visibility or ensure the playlist DOM has the expected structure before mutating it.
Server/webbyBits.py
@ -71,0 +74,4 @@
socketio.emit("songAdd",getSongInfo(song))
def getSongInfo(song):
fileofDB = sql.connect("songDatabase.db")
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:36 -05:00
Copy link

cors_allowed_origins="*" enables any website to open a socket connection to this server and receive real-time events (playlist/time/settings). If this is intended only for the hosted client, consider making allowed origins configurable (e.g., via .env) and defaulting to the client origin instead of *.

# Configure allowed origins for Socket.IO via environment variable to avoid allowing all origins by default.
cors_allowed_origins_env = os.getenv("CORS_ALLOWED_ORIGINS")
if cors_allowed_origins_env:
    cors_allowed_origins = [origin.strip() for origin in cors_allowed_origins_env.split(",") if origin.strip()]
else:
    # Default to None (no cross-origin access) instead of "*" for better security.
    cors_allowed_origins = None
socketio = SocketIO(app, cors_allowed_origins=cors_allowed_origins)
`cors_allowed_origins="*"` enables any website to open a socket connection to this server and receive real-time events (playlist/time/settings). If this is intended only for the hosted client, consider making allowed origins configurable (e.g., via `.env`) and defaulting to the client origin instead of `*`. ```suggestion # Configure allowed origins for Socket.IO via environment variable to avoid allowing all origins by default. cors_allowed_origins_env = os.getenv("CORS_ALLOWED_ORIGINS") if cors_allowed_origins_env: cors_allowed_origins = [origin.strip() for origin in cors_allowed_origins_env.split(",") if origin.strip()] else: # Default to None (no cross-origin access) instead of "*" for better security. cors_allowed_origins = None socketio = SocketIO(app, cors_allowed_origins=cors_allowed_origins) ```
readme.md
copilot-pull-request-reviewer[bot] (Migrated from github.com) commented 2026-02-11 09:28:38 -05:00
Copy link

Spelling: "seperate" should be "separate".

                    - This is a separate toggle, but is based on the setting "Add track to queue"
Spelling: "seperate" should be "separate". ```suggestion - This is a separate toggle, but is based on the setting "Add track to queue" ```
Sign in to join this conversation.
Reviewers
No reviewers
Labels
Clear labels   
bug

Something isn't working

  
documentation

Improvements or additions to documentation

  
duplicate

This issue or pull request already exists

  
enhancement

New feature or request

  
good first issue

Good for newcomers

  
help wanted

Extra attention is needed

  
invalid

This doesn't seem right

  
question

Further information is requested

  
wontfix

This will not be worked on

No labels
bug
documentation
duplicate
enhancement
good first issue
help wanted
invalid
question
wontfix
Milestone
Clear milestone
No items
No milestone
Projects
Clear projects
No items
No project
Assignees
Clear assignees
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: KristyF/PartyJukebox#7
No description provided.
Delete branch "timerChanges"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?