Open
Conversation
There was a problem hiding this comment.
Actionable comments posted: 9
🤖 Fix all issues with AI agents
In `@internal/frontend/gallery.go`:
- Around line 37-39: The comment above the pagedata check in ssrGallery refers
to a non-existent "synchronized block" and is misleading; update or remove that
comment in internal/frontend/gallery.go so it correctly reflects ssrGallery's
behavior (e.g., explain that pagedata == nil indicates prior
error/initialization failure and thus should skip templating) or simply delete
the sentence referencing synchronization to avoid confusion.
- Around line 28-35: ssrGallery currently builds galleryPageData and renders
without validating that the lobby exists; add a lookup using
state.GetLobby(lobbyId) at the start of ssrGallery and if it returns nil call
handler.userFacingError(writer, translation.Get("lobby-doesnt-exist"),
translation) then return to avoid rendering the page for a non-existent lobby.
Ensure you reference the same lobbyId variable and perform this check before
creating galleryPageData or writing the response so client-side fetches won't
fail.
In `@internal/frontend/gallery.js`:
- Around line 80-82: The console log in the else branch is using the wrong
variable name; change the log to reference drawElement.type (the same variable
used in the preceding type checks) instead of drawData.type so the message
accurately reflects the evaluated element; update the log statement in the
function handling draw elements (look for the else branch that currently logs
"Unknown draw element type: " + drawData.type) to use drawElement.type.
- Around line 9-32: The getGallery function uses a static sessionStorage key
("cached_gallery") causing cross-lobby collisions and doesn't handle non-OK HTTP
responses or JSON parse errors; update getGallery to derive the cache key from
the lobby ID (e.g. include {{.LobbyID}} in the key instead of "cached_gallery"),
use that derived key when calling sessionStorage.getItem()/setItem(), and
enhance the fetch flow to check response.ok and reject with a meaningful error
when not ok, and catch JSON.parse failures (or wrap response.json() in a
try/catch) so parsing errors also reject the promise; refer to getGallery, the
current sessionStorage.getItem("cached_gallery")/setItem("cached_gallery")
usages, and the fetch("{{.RootPath}}/v1/lobby/{{.LobbyID}}/gallery") call to
locate where to apply these changes.
- Around line 91-94: The getGallery() call lacks rejection handling and doesn't
guard against an empty array; update the promise chain around getGallery() to
add a .catch handler to log/display errors and ensure you only call
setDrawing(data[0]) when data is an array with length > 0 (otherwise set a safe
default or skip setDrawing), while still assigning galleryData = data (or [] on
error) so downstream code has a consistent state; refer to getGallery,
setDrawing, and galleryData when making these changes.
In `@internal/frontend/resources/gallery.css`:
- Around line 37-41: Remove the empty CSS rule blocks for the .prev and .next
selectors by deleting their empty declarations (the ".prev { }" and ".next { }"
blocks) from the stylesheet; if they are intended as placeholders, replace them
with a single-line comment indicating that (e.g., /* TODO: styles for .prev */)
instead of leaving empty rules.
In `@internal/frontend/templates/gallery.html`:
- Around line 7-10: The viewport meta tag in gallery.html has a syntax error:
the content attribute in the <meta name="viewport"> element is missing a comma
between initial-scale=1 and maximum-scale=1; update the content string for the
viewport meta so parameters are comma-separated (e.g., include a comma between
initial-scale=1 and maximum-scale=1) to ensure consistent parsing across
browsers.
- Around line 32-36: Replace the hardcoded "Prev" and "Next" labels in the
template's top-bar buttons (elements with id="prev" and id="next") with calls to
the existing translation helper, e.g. use {{.Translation.Get "gallery-prev"}}
and {{.Translation.Get "gallery-next"}}—update the <button> contents to use
these translation keys so the labels are internationalized and add corresponding
keys to the translation files.
In `@internal/game/data.go`:
- Around line 84-86: Fix the typo in the struct comment above the Drawings
field: change "accross" to "across" in the comment that documents Drawings
[]GalleryDrawing (the comment starting "// Drawings contains the history...").
Ensure the updated wording reads "across all rounds and games played." to keep
the comment clear and correct.
Member
Author
|
The new plan:
|
1aaf47e to
3e76352
Compare
3e76352 to
3f2177e
Compare
* add drawer name to data and frontend * make layout more compact * add menu entry to load gallery
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Branch contains a demo that allows you to navigate to
lobby/id/galleryto see the drawings:Currently it's cached, so it doesn't refresh at all.
I'd say we should somehow place this in the lobby. Maybe simply in the menu which then opens a new tab?
I'd say across rounds in a lobby, the drawings should be kept. Given the current usage patterns of the application, memory usage should not be an issue.
Once all players have disconnected, we discard the lobby.
We can automatically download and cache the gallery once a round ends.
Todos:
As we currently just replay the drawing, we could technically do the same with the chat at some point, but that would be a bonus!