Conversation
* Add keyboard layout selection and update keyboard configurations for multiple languages * Refactor keyboard layout selector form for improved styling
WalkthroughAdds multi-layout keyboard support: normalizes keyboard data, exposes multiple named layouts, selects an active layout per request (query or cookie), auto-generates an alphabetical layout when none provided, and surfaces a layout selector in the game settings. Changes
Sequence Diagram(s)sequenceDiagram
actor User
participant Browser
participant App as Flask App
participant Lang as Language
participant UI as game.html
User->>Browser: Request /language?code=en&layout=dvorak
Browser->>App: GET /language?code=en&layout=dvorak
rect rgb(230,240,255)
Note over App: Load & normalize keyboard data
App->>App: load_keyboard(config) => {default, layouts}
App->>Lang: Language(code, word_list, keyboard_layout=dvorak)
Lang->>Lang: _build_keyboard_layouts(config)
Lang->>Lang: _select_keyboard_layout(dvorak)
Lang->>Lang: set self.keyboard (rows, label, name)
end
App->>Browser: Set-Cookie: keyboard_layout=dvorak
App->>UI: render with Language instance
UI->>Browser: render selector if multiple layouts
Browser->>App: POST/GET with new layout (user change)
App->>Browser: Update cookie + render updated UI
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
webapp/app.py (1)
123-126: Narrow the exception scope inload_keyboard.
Catching bareExceptionhides unrelated errors (e.g., OS errors) when loading keyboard data, making failures harder to diagnose. Limit this tojson.JSONDecodeErrorand expected I/O errors instead of swallowing everything.- except Exception: + except json.JSONDecodeError: + return {"default": None, "layouts": {}}If you also want to cover filesystem issues, catch
OSErrorexplicitly in a separate clause.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
webapp/app.py(4 hunks)webapp/data/languages/en/en_keyboard.json(1 hunks)webapp/data/languages/he/he_keyboard.json(1 hunks)webapp/templates/game.html(2 hunks)
🧰 Additional context used
🪛 Ruff (0.14.4)
webapp/app.py
125-125: Do not catch blind exception: Exception
(BLE001)
…sibility in game.html
|
@coderabbitai generate docstrings |
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Warning Docstrings generation - IN PROGRESS Generating docstrings for this pull request |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
webapp/app.py (3)
118-167: Keyboard loading: blindexcept Exceptionhides real errors and hinders debuggingThe overall normalization logic looks solid and safely handles legacy list configs and new multi-layout dicts, but the error handling is too broad:
except Exception:(Line 131) swallows any runtime problem (JSON syntax errors, permission issues, programming mistakes) and silently falls back to an empty layout, which then auto-generates a keyboard. This makes it hard to notice broken keyboard JSON and can mask genuine bugs. Static analysis (BLE001) is flagging exactly this.I recommend:
- Narrow the second handler to the expected failure modes (e.g. JSON decoding and I/O):
- except FileNotFoundError: - return {"default": None, "layouts": {}} - except Exception: - return {"default": None, "layouts": {}} + except FileNotFoundError: + return {"default": None, "layouts": {}} + except (OSError, json.JSONDecodeError) as err: + # Optional: log so malformed keyboard configs are visible in logs. + app.logger.warning("Failed to load keyboard %s: %s", keyboard_path, err) + return {"default": None, "layouts": {}}
- If you want to be stricter during development, you could re‑raise unexpected exceptions instead of treating them as “no keyboard”.
245-297: Language keyboard layout initialization is robust; consider minimal defensive checkThe layout initialization in
Language.__init__and_build_keyboard_layoutsis cohesive and resilient:
- It always produces at least one layout by auto‑generating an alphabetical layout when
keyboard_config["layouts"]is empty.- It cleanly separates concerns:
load_keyboardnormalizes raw JSON;_build_keyboard_layoutsenforces a canonical{"label", "rows"}shape;_select_keyboard_layouthandles request/default precedence.Given that
_build_keyboard_layoutsguarantees at least one layout today,self.keyboard_layouts[self.keyboard_layout_name]is safe. If you ever relax that guarantee, a small guard would prevent a future KeyError:- self.keyboard_layouts = self._build_keyboard_layouts(keyboard_config) - self.keyboard_layout_name = self._select_keyboard_layout( - keyboard_layout, keyboard_config.get("default") - ) - layout_meta = self.keyboard_layouts[self.keyboard_layout_name] + self.keyboard_layouts = self._build_keyboard_layouts(keyboard_config) + if not self.keyboard_layouts: + # Should not happen with current _build_keyboard_layouts contract, + # but avoids a KeyError if that ever changes. + self.keyboard_layouts = { + "alphabetical": { + "label": "Alphabetical", + "rows": self._generate_alphabetical_keyboard(), + } + } + self.keyboard_layout_name = self._select_keyboard_layout( + keyboard_layout, keyboard_config.get("default") + ) + layout_meta = self.keyboard_layouts[self.keyboard_layout_name]If you’re confident in the current contract, this can be considered optional; the existing logic cleanly satisfies the multi-layout requirement and fallback behavior.
390-402: Layout selection & cookie persistence: behavior aligns with requirements; consider minor cookie hardeningThe updated
languageroute correctly:
- Reads the requested layout from
?layout=with precedence over the cookie (keyboard_layout_<lang>), then delegates toLanguagewhich safely falls back when the value is unknown.- Sets the cookie only when the effective
language.keyboard_layout_namechanges, avoiding unnecessary rewrites.- Uses a 1‑year
max_ageandsamesite="Lax", which is reasonable for a per-user preference.Two small optional refinements:
- Add
secure=Truetoset_cookieto match the HTTPS redirect behavior inbefore_request, preventing this preference from ever being sent over plain HTTP in case of misconfiguration.- If you ever care about explicit “reset to default” semantics, you might prefer checking for
"layout" in request.argsinstead of falsy/truthy, so an emptylayoutvalue doesn’t unintentionally fall back to the cookie.Example hardened cookie call:
- response.set_cookie( + response.set_cookie( cookie_key, selected_layout, max_age=60 * 60 * 24 * 365, - samesite="Lax", + samesite="Lax", + secure=True, )
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
webapp/app.py(4 hunks)webapp/templates/game.html(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- webapp/templates/game.html
🧰 Additional context used
🪛 Ruff (0.14.4)
webapp/app.py
131-131: Do not catch blind exception: Exception
(BLE001)
🔇 Additional comments (1)
webapp/app.py (1)
299-334: Alphabetical keyboard generation and row balancing look correctThe selection and generation helpers behave as intended:
_select_keyboard_layoutcorrectly prioritizes user-requested layout, then config default, then the first available layout, ensuring an always-valid choice given the_build_keyboard_layoutsguarantee._generate_alphabetical_keyboard:
- Uses
self.characters(already filtered to only used characters) to build 10-column rows.- Safely adds the “⇨” and “⌫” keys and redistributes characters when the last row is overly long (11 or 12 entries), without risking index errors thanks to the explicit length checks.
This should produce stable, readable fallback layouts for languages without explicit keyboard configs, and the logic appears safe for varying alphabet sizes.
✅ Actions performedInitiated docstring generation; will generate only if new commits exist. |
|
Hello @Hugo0 ! I imagine you're super busy! What do you think about this feature/implementation? Any chance this could be merged? Did this for a non technical friend of mine and currently hosting this branch for him on a legacy free render instance that turns itself off pretty often. Lemme know if this isn't the direction you see for this app and I'll figure something else out if it isn't. Thank you! |
|
Thanks @professor-eggs! Super sorry for the late response here. Insane that you're hosting a fork; you're a good friend. I've now incorporated your changes in commits 8f59b7f and 8b1b3f5 in PR #109, along with a bunch of other stuff. Let me know if you like the direction of the app! |
Integrate professor-eggs' multiple keyboard layouts feature:
- New keyboard JSON format with named layouts (qwerty, dvorak, alphabetical)
- Backward compatible with legacy array format
- User layout preference persisted via cookies
- Layout selector in Settings modal (only shows when multiple layouts exist)
- Auto-generated alphabetical fallback when no keyboard defined
Changes:
- load_keyboard() now returns normalized {default, layouts} structure
- Language class handles layout selection with fallback chain
- English keyboard: QWERTY, Dvorak, Alphabetical layouts
- Hebrew keyboard: Alphabetical, Hebrew QWERTY layouts
- Added dark mode styling to layout selector
Based on PR #105 by @professor-eggs, adapted for current codebase.
Co-Authored-By: professor-eggs <professor-eggs@users.noreply.github.com>
2 participants
This pull request introduces support for multiple selectable keyboard layouts per language in game. The main changes include refactoring the keyboard loading logic, updating the language model to handle multiple layouts, adding new keyboard layout data for English and Hebrew, and providing a UI for users to select their preferred keyboard layout, which is persisted via cookies.
Keyboard Layout System Enhancements:
load_keyboardinwebapp/app.pyto support both legacy array and new object-based keyboard config formats, normalizing layouts and handling missing or malformed files gracefully.Languageclass inwebapp/app.pyto support multiple keyboard layouts, including logic for selecting the active layout, generating an alphabetical fallback, and storing layout metadata. [1] [2]Keyboard Data Updates:
webapp/data/languages/en/en_keyboard.json) with a new format supporting "qwerty", "dvorak", and "alphabetical" layouts. [1] [2]webapp/data/languages/he/he_keyboard.json) to provide "alphabetical" and "Hebrew QWERTY" layouts in the new format.User Interface Improvements:
game.html, allowing users to switch layouts when more than one is available.Closes #106
Summary by CodeRabbit