feat(game-lobby): auto-focus on player name input #860
Conversation
|
Caution Review failedThe pull request is closed. WalkthroughThis pull request introduces updates to the Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
Tip Early access features: enabledWe are currently testing the following features in early access:
Note:
|
![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
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (3)
tests/acceptance/screenshots/darwin/Game Lobby Page without players.pngis excluded by!**/*.pngtests/acceptance/screenshots/linux/Game Lobby Page without players.pngis excluded by!**/*.pngtests/unit/specs/components/pages/game-lobby/GameLobbyHeader/GameLobbyPlayerInput/__snapshots__/GameLobbyPlayerInput.nuxt.spec.ts.snapis excluded by!**/*.snap
Files selected for processing (12)
- .gitignore (2 hunks)
- app/components/pages/game-lobby/GameLobbyHeader/GameLobbyPlayerInput/GameLobbyPlayerInput.vue (2 hunks)
- app/composables/misc/useDevice.ts (1 hunks)
- config/eslint/flat-configs/eslint.global-config.mjs (1 hunks)
- env/.env (1 hunks)
- modules/i18n/i18n.config.ts (1 hunks)
- nuxt.config.ts (1 hunks)
- tests/acceptance/features/game-lobby/features/game-lobby.feature (1 hunks)
- tests/acceptance/features/playwright/step-definitions/inputs/playwright-inputs.then-steps.ts (1 hunks)
- tests/acceptance/features/support/hooks.ts (1 hunks)
- tests/unit/specs/components/pages/game-lobby/GameLobbyHeader/GameLobbyPlayerInput/GameLobbyPlayerInput.nuxt.spec.ts (1 hunks)
- tests/unit/specs/composables/misc/useDevice.spec.ts (1 hunks)
Additional context used
Biome
tests/unit/specs/composables/misc/useDevice.spec.ts
[error] 9-9: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
[error] 13-13: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
Additional comments not posted (21)
modules/i18n/i18n.config.ts (1)
10-10: LGTM!The addition of the
defaultLocaleproperty is a good practice as it explicitly defines the default locale for the i18n setup. Setting it to the same value aslocaleensures consistency in the locale configuration. The change aligns well with the existing code structure and follows the established pattern of usingruntimeConfig.public.defaultLocalefor locale configuration.app/composables/misc/useDevice.ts (1)
7-13: LGTM!The
useDevicecomposable function is correctly implemented and follows the Vue 3 Composition API pattern. The computed propertyisOnTouchDeviceuses theontouchstartproperty andnavigator.maxTouchPointsto determine if the device supports touch events. This composable can be used in other components to conditionally render UI elements based on the device type..gitignore (1)
5-5: LGTM!The addition of
.nuxt-test-cucumberto the.gitignorefile is appropriate as it ensures that test-related artifacts specific to Cucumber testing within the Nuxt.js framework are not tracked by Git. This change aligns with the best practice of excluding generated files and test artifacts from version control.tests/acceptance/features/playwright/step-definitions/inputs/playwright-inputs.then-steps.ts (1)
26-27: LGTM!The new step definition is correctly implemented and enhances the testing capabilities by enabling the validation of focus state. The regex pattern is correctly defined to capture the label of the input field, and the function uses the appropriate Playwright methods to locate the input field and check if it is focused. The changes are in line with the PR objective to auto-focus on the player name input.
tests/unit/specs/composables/misc/useDevice.spec.ts (4)
6-10: Test setup and teardown code looks good!The test setup and teardown code correctly mocks the
navigator.maxTouchPointsproperty and resets thewindow.ontouchstartproperty between test cases.Regarding the static analysis hints about the usage of the
deleteoperator:
- The
deleteoperator is used in a test environment and not in production code. Therefore, the performance impact is negligible.- Deleting the
window.ontouchstartproperty is necessary to reset the state between test cases.Also applies to: 12-14
Tools
Biome
[error] 9-9: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
16-21: Test case looks good!The test case correctly tests the behavior of the
isOnTouchDevicecomputed property when thewindow.ontouchstartproperty is available.
23-29: Test case looks good!The test case correctly tests the behavior of the
isOnTouchDevicecomputed property when thenavigator.maxTouchPointsproperty is greater than 0.
31-35: Test case looks good!The test case correctly tests the behavior of the
isOnTouchDevicecomputed property when neither thewindow.ontouchstartproperty nor thenavigator.maxTouchPointsproperty is available.tests/acceptance/features/support/hooks.ts (4)
14-14: LGTM!Using a separate build directory for tests is a good practice to isolate the test artifacts from the main build.
16-29: LGTM!Configuring the browser options and environment variables ensures that the tests run in a controlled environment that mimics production settings. This is crucial for the reliability and reproducibility of the tests.
34-34: LGTM!Setting
skipSettingLocaleOnNavigatetotrueensures that the locale is not changed when navigating between pages during tests. This is important for maintaining a consistent locale throughout the test scenarios.
35-35: LGTM!Specifying a single locale for testing ensures that the application behaves correctly in a localized context during tests. This helps to catch any locale-specific issues and ensures that the internationalization functionality works as expected.
app/components/pages/game-lobby/GameLobbyHeader/GameLobbyPlayerInput/GameLobbyPlayerInput.vue (3)
12-12: LGTM!The conditional
autofocusattribute based on the device type is a nice touch to enhance the user experience. The usage of theuseDevicecomposable is correct.
59-59: LGTM!The
useDevicecomposable is correctly imported, and the import statement follows the existing import order.
66-67: LGTM!The
isOnTouchDeviceproperty is correctly destructured from theuseDevicecomposable, and the destructuring assignment follows the existing code structure.nuxt.config.ts (3)
92-92: LGTM!Retrieving the default locale from an environment variable is a good practice. It allows for easier configuration and deployment across different environments.
94-95: Looks good!The removal of unnecessary line breaks in the
localesarray improves the readability of the configuration without affecting the functionality.
100-100: Verify the impact on image sources.Reducing the
domainsarray to a single entry may affect the sources from which images can be loaded. Please ensure that all the required image sources are still included after this change.You can run the following script to search for image URLs in the codebase and verify that they are all from the allowed domain:
If the script outputs any image URLs, please review them and consider adding the necessary domains to the
domainsarray.Verification successful
No impact on production image handling.
The image URLs found in the codebase are from
http://127.0.0.1and are located in test files. These are likely used for testing purposes and do not affect the application's image handling in production. The change to thedomainsarray innuxt.config.tsis appropriate.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for image URLs in the codebase and verify the domains. # Test: Search for image URLs in HTML, CSS, and JavaScript files. # Expect: All image URLs should be from the allowed domain "werewolves-assistant.com". rg --type-add 'web:*.{html,css,js,ts,vue}' -t web -o $'https?://[^"\' ]+\.(png|jpe?g|gif|svg|webp|ico|bmp)' | rg -v $'werewolves-assistant\.com'Length of output: 432
config/eslint/flat-configs/eslint.global-config.mjs (1)
11-11: LGTM!Adding
navigatorto the list of global variables in the ESLint configuration is a good practice, as it prevents ESLint from flagging the usage ofnavigatoras an undefined variable. Marking it asREADONLYis also a good practice, as it prevents accidental modification of thenavigatorobject.tests/unit/specs/components/pages/game-lobby/GameLobbyHeader/GameLobbyPlayerInput/GameLobbyPlayerInput.nuxt.spec.ts (1)
34-38: LGTM!The new test case is a great addition to improve the test coverage and ensure the expected user experience by verifying that the input field is automatically focused when the component is rendered.
tests/acceptance/features/game-lobby/features/game-lobby.feature (1)
13-13: LGTM!The new step definition correctly checks if the player name input is focused when the game lobby page is displayed. This change enhances the user experience and aligns with the PR objective.
env/.env
Outdated
| NUXT_PUBLIC_WEREWOLVES_ASSISTANT_API_BASE_URL="http://127.0.0.1:9090" | ||
| NUXT_PUBLIC_DEFAULT_LOCALE="en" | ||
| NUXT_SITE_URL="http://127.0.0.1:3000" | ||
| NUXT_SITE_NAME="Werewolves Assistant" | ||
| NUXT_SITE_ENV="development" | ||
| NUXT_SITE_DESCRIPTION="The perfect tool for game masters of the Werewolves of Miller's Hollow™" |
There was a problem hiding this comment.
Security Issue: Don't commit sensitive information. Use .env.example instead.
The .env file contains sensitive information such as internal API URLs and environment-specific details. Committing this file to version control poses security risks as it exposes potentially confidential data.
Best practices:
- Create a
.env.examplefile with the necessary variable names and example values. Commit this file to the repository as a template. - Add the actual
.envfile to your.gitignoreto prevent accidental commits of sensitive data. - Provide instructions in the project's README on how to set up the
.envfile for development.
By following these practices, you maintain the security of your environment variables while providing a clear setup guide for other developers.
On a positive note, the variable naming conventions follow the NUXT_ prefix, which is a good practice for Nuxt.js projects. The variable names are descriptive and the default values provide fallbacks, enhancing the developer experience.
|
## [1.32.0](v1.31.0...v1.32.0) (2024-09-18) ### 🚀 Features * **audio:** audio settings in local storage ([#831](#831)) ([cd8a2cb](cd8a2cb)) * **game-lobby:** all game lobby is responsive ([#877](#877)) ([aaccf38](aaccf38)) * **game-lobby:** auto-focus on player name input ([#860](#860)) ([7897da3](7897da3)) * **game-lobby:** auto-focus on player name input even after changing page ([#873](#873)) ([3bb8401](3bb8401)) * **game-lobby:** display disclaimer for small screens ([#853](#853)) ([81029b8](81029b8)) * **game-lobby:** game options confirm step in lobby ([#845](#845)) ([20cc2ce](20cc2ce)) * **game:** all game screens are responsive ([#880](#880)) ([b54b1dc](b54b1dc)) * **game:** game over screens are responsive ([#883](#883)) ([aacb761](aacb761)) * **misc-pages:** responsive for canceled game and not found game pages ([#888](#888)) ([deeb7b2](deeb7b2)) * **nuxt:** set route announcer ([#835](#835)) ([dd1115f](dd1115f)) * **nuxt:** typecheck on hooks and ci ([#832](#832)) ([794157f](794157f)) ### 🐛 Bug Fixes * **deps:** update dependency @primevue/themes to ^4.0.6 ([#842](#842)) ([4d3e063](4d3e063)) * **deps:** update dependency @primevue/themes to ^4.0.7 ([#855](#855)) ([2b80485](2b80485)) * **deps:** update dependency primevue to ^4.0.6 ([#843](#843)) ([3a20f01](3a20f01)) * **deps:** update dependency primevue to ^4.0.7 ([#856](#856)) ([6c75a61](6c75a61)) * **game-events:** icon animation null instance handler ([#889](#889)) ([ee0a17e](ee0a17e)) * **pwa:** remove redirect fallback ([#830](#830)) ([69c2c21](69c2c21)) ### 🔩 Refactor * **primevue:** use new accordion component ([#834](#834)) ([f611995](f611995)) * **primevue:** use new tab components ([#836](#836)) ([4f9c1b9](4f9c1b9)) ### ✅ Tests * **scapegoat:** acceptance tests ([#884](#884)) ([add895e](add895e)) ### 🔁 CI * **node:** use last node version in ci ([#848](#848)) ([aa047b3](aa047b3)) ### 🧹 Chore * **deps:** update commitlint monorepo to ^19.5.0 ([#852](#852)) ([cefe4af](cefe4af)) * **deps:** update dependency @cucumber/cucumber to ^11.0.1 ([#869](#869)) ([c00d3e7](c00d3e7)) * **deps:** update dependency @cucumber/cucumber to v11 ([#817](#817)) ([dcbad46](dcbad46)) * **deps:** update dependency @faker-js/faker to ^9.0.1 ([#870](#870)) ([da15f7c](da15f7c)) * **deps:** update dependency @nuxt/test-utils to ^3.14.2 ([#846](#846)) ([f0f715a](f0f715a)) * **deps:** update dependency @nuxtjs/i18n to ^8.5.2 ([#764](#764)) ([0fe9bf0](0fe9bf0)) * **deps:** update dependency @nuxtjs/i18n to ^8.5.3 ([#849](#849)) ([1689312](1689312)) * **deps:** update dependency @nuxtjs/robots to ^4.1.7 ([#861](#861)) ([4cf88c8](4cf88c8)) * **deps:** update dependency @primevue/nuxt-module to ^4.0.6 ([#841](#841)) ([7384c1e](7384c1e)) * **deps:** update dependency @primevue/nuxt-module to ^4.0.7 ([#854](#854)) ([22c3f4a](22c3f4a)) * **deps:** update dependency @stylistic/eslint-plugin to ^2.8.0 ([#838](#838)) ([1c15410](1c15410)) * **deps:** update dependency @types/qs to ^6.9.16 ([#868](#868)) ([8560a5d](8560a5d)) * **deps:** update dependency @vueuse/core to ^11.1.0 ([#875](#875)) ([b568680](b568680)) * **deps:** update dependency @vueuse/nuxt to ^11.1.0 ([#876](#876)) ([0f358b6](0f358b6)) * **deps:** update dependency circle-progress.vue to ^3.2.2 ([#878](#878)) ([4cd09ac](4cd09ac)) * **deps:** update dependency husky to ^9.1.6 ([#858](#858)) ([03b278b](03b278b)) * **deps:** update dependency msw to ^2.4.3 ([#827](#827)) ([6cd07ef](6cd07ef)) * **deps:** update dependency msw to ^2.4.4 ([#833](#833)) ([44814db](44814db)) * **deps:** update dependency msw to ^2.4.5 ([#850](#850)) ([773674a](773674a)) * **deps:** update dependency msw to ^2.4.6 ([#864](#864)) ([93b4a45](93b4a45)) * **deps:** update dependency msw to ^2.4.7 ([#871](#871)) ([b6cef68](b6cef68)) * **deps:** update dependency msw to ^2.4.8 ([#882](#882)) ([96d9d56](96d9d56)) * **deps:** update dependency node to v22.9.0 ([#886](#886)) ([8f28e99](8f28e99)) * **deps:** update dependency sass to ^1.79.1 ([#887](#887)) ([02e728a](02e728a)) * **deps:** update dependency semantic-release to ^24.1.1 ([#851](#851)) ([9a5d974](9a5d974)) * **deps:** update dependency tailwindcss to ^3.4.11 ([#857](#857)) ([74668ab](74668ab)) * **deps:** update dependency tailwindcss to ^3.4.12 ([#885](#885)) ([b2d182d](b2d182d)) * **deps:** update dependency tsx to ^4.19.1 ([#859](#859)) ([6e771c3](6e771c3)) * **deps:** update dependency type-fest to ^4.26.1 ([#829](#829)) ([7b715fd](7b715fd)) * **deps:** update dependency typescript to ^5.6.2 ([#839](#839)) ([69b99b1](69b99b1)) * **deps:** update dependency vue to ^3.5.4 ([#844](#844)) ([09fd254](09fd254)) * **deps:** update dependency vue to ^3.5.5 ([#865](#865)) ([13ae2b7](13ae2b7)) * **deps:** update dependency vue to ^3.5.6 ([#874](#874)) ([77302a2](77302a2)) * **deps:** update dependency vue-router to ^4.4.4 ([#847](#847)) ([23430aa](23430aa)) * **deps:** update dependency vue-router to ^4.4.5 ([#863](#863)) ([28a6c29](28a6c29)) * **deps:** update nuxtjs monorepo to ^3.13.2 ([#872](#872)) ([28e0e43](28e0e43)) * **deps:** update playwright monorepo to ^1.47.1 ([#866](#866)) ([ae9e014](ae9e014)) * **deps:** update pnpm to v9.10.0 ([#837](#837)) ([05be262](05be262)) * **deps:** update typescript-eslint monorepo to ^8.5.0 ([#840](#840)) ([b230013](b230013)) * **deps:** update typescript-eslint monorepo to ^8.6.0 ([#881](#881)) ([c291635](c291635)) * **deps:** update vitest monorepo to ^2.1.0 ([#862](#862)) ([b25b312](b25b312)) * **deps:** update vitest monorepo to ^2.1.1 ([#867](#867)) ([a1ad2f8](a1ad2f8)) * **screenshots:** missing screenshots for linux ([8bfa635](8bfa635))
|
🎉 This PR is included in version 1.32.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
Summary by CodeRabbit
New Features
Configuration Changes
.gitignoreto exclude test-related artifacts and modified the ignored environment file.Bug Fixes
Tests