moving intl deeper into the toolkit

toddtarsi commented 2 months ago

User description

@fernandozw - I am reworking the i18n stuff a bit for the following tweaks:

I am moving some of the command map translations into side-model. I want that stuff really low down so we can reuse your translations cleanly in both the IDE and the side runner. I am assuming if you're enjoying the IDE, you'll probably want correctly translated strings for your CI too.
I am moving the app translation hooks to just use react-intl and a single typed master translation document. This way we get a few more errors about missing translations, and whenever I add a new english string, the compiler will kick my ass until I add a chinese equivalent. It will help me keep language support going forward. Also, react-intl has good support for template strings and variable injection w/r/t localization and doing numbers and dates and all that stuff too.
Rather than manually specifying language and storing that in the electron-store, I'm just going to use the system locale data so the app starts in the users language of choice.
I am looking at combining metadata from custom commands into the language map in the browser at runtime. Right now, custom commands (provided via plugin) crash the app UX.

Type

enhancement

Description

Refactored various components to use dynamic command maps and localized language maps from @seleniumhq/side-model.
Enhanced internationalization support across the application by integrating IntlProvider and updating components to use FormattedMessage.
Added comprehensive language support files for English and Chinese, including UI labels and command descriptions.
Improved code quality and maintainability by updating components to use modern React hooks and patterns.

Changes walkthrough

Relevant files

Enhancement

11 files

LocatorField.tsx `Refactor LocatorField to Use Dynamic Command Maps and Locale` packages/selenium-ide/src/browser/windows/ProjectEditor/tabs/Tests/CommandFields/LocatorField.tsx Integrated `Commands` from `@seleniumhq/side-model` to replace hardcoded command maps. Utilized `useState` and `useEffect` to dynamically update language maps and local values based on system locale. Improved function handling and state management for UI components.	+119/-1122
TextField.tsx `Update TextField to Utilize Dynamic Commands and Language Maps` packages/selenium-ide/src/browser/windows/ProjectEditor/tabs/Tests/CommandFields/TextField.tsx Replaced static command definitions with dynamic imports from `@seleniumhq/side-model`. Enhanced state management with `useState` and `useEffect` for language and command updates.	+28/-909
index.ts `Add Chinese Translations for UI Components and Commands` packages/selenium-ide/src/browser/I18N/zh/index.ts Added comprehensive Chinese translations for various UI components and menus. Integrated commands and argument types translations from `@seleniumhq/side-model`.	+225/-0
index.ts `Setup Base English Translations and Dynamic Command Imports` packages/selenium-ide/src/browser/I18N/en/index.ts Established a base English translation file for UI components and system messages. Included dynamic command and argument types from `@seleniumhq/side-model`.	+222/-0
ArgTypes.ts `Define Chinese Argument Types for Localization` packages/side-model/src/I18N/zh/ArgTypes.ts Defined argument types for commands in Chinese to support localization.	+155/-0
AppBarTabs.tsx `Refactor AppBarTabs for Internationalization Support` packages/selenium-ide/src/browser/components/AppBar/AppBarTabs.tsx Updated component to use formatted messages for internationalization.	+26/-28
RenamableListItem.tsx `Enhance RenamableListItem with forwardRef Implementation` packages/selenium-ide/src/browser/components/Drawer/RenamableListItem.tsx Modified to use `forwardRef` for better ref handling in React components.	+7/-11
AppWrapper.tsx `Wrap App Components with IntlProvider for Localization` packages/selenium-ide/src/browser/components/AppWrapper.tsx Wrapped application components within `IntlProvider` for internationalization.	+9/-8
index.ts `Implement I18N Data Loader for Language Support` packages/side-model/src/I18N/index.ts Created a loader for internationalization data based on language settings.	+33/-0
keys.ts `Generate I18N Keys for Language Mapping` packages/selenium-ide/src/browser/I18N/keys.ts Generated keys for internationalization to map language-specific labels.	+24/-0
ArgTypes.ts `Export Default English Argument Types` packages/side-model/src/I18N/en/ArgTypes.ts - Exported default argument types for commands in English.	+3/-0

✨ PR-Agent usage: Comment /help on the PR to get a list of all available PR-Agent tools and their descriptions

codiumai-pr-agent-pro[bot] commented 2 months ago

CI Failure Feedback

**Action:** test

**Failed stage:** [Build side-runner and selenium-ide](https://github.com/SeleniumHQ/selenium-ide/actions/runs/8848492490/job/24298426571) [❌]

**Failure summary:** The action failed due to multiple TypeScript compilation errors across various packages. Key issues
include:

Missing index signatures in TypeScript types, causing TS7053 errors in multiple files such as
packages/side-runtime/src/plugins.ts and packages/side-migrate/src/migrations/implicit-locators.ts.

Failed attempts to create binary links due to missing directories or files, as indicated by ENOENT
errors in paths like
/home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-commons/node_modules/.bin/side-code-export.

Dependencies between projects not being met, leading to skipped builds. For example,
packages/side-code-export could not be built due to errors in its dependency packages/side-runtime,
which cascaded and affected other dependent projects.

Relevant error logs:

```yaml 1: ##[group]Operating System 2: Ubuntu ... 176: .../node_modules/@bazel/hide-bazel-files postinstall: Done 177: .../node_modules/@bazel/typescript postinstall: Done 178: .../node_modules/electron postinstall$ node install.js 179: .../node_modules/electron postinstall$ node install.js 180: .../node_modules/electron-chromedriver install$ node ./download-chromedriver.js 181: .../node_modules/electron postinstall: Done 182: .../node_modules/electron postinstall: Done 183: .../node_modules/electron-chromedriver install: Done 184: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-commons/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 185: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-nunit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 186: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-java-junit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 187: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-xunit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 188: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-python-pytest/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 189: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-javascript-mocha/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 190: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/selenium-ide/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 191: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/side-example-suite/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 192: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-ruby-rspec/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/dist/bin.js' 193: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-java-junit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-java-junit/node_modules/side-code-export/dist/bin.js' 194: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-commons/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-commons/node_modules/side-code-export/dist/bin.js' 195: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-python-pytest/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-python-pytest/node_modules/side-code-export/dist/bin.js' 196: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-ruby-rspec/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-ruby-rspec/node_modules/side-code-export/dist/bin.js' 197: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-javascript-mocha/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-javascript-mocha/node_modules/side-code-export/dist/bin.js' 198: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-nunit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-nunit/node_modules/side-code-export/dist/bin.js' 199: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-xunit/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-csharp-xunit/node_modules/side-code-export/dist/bin.js' 200: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/side-example-suite/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/side-example-suite/node_modules/side-code-export/dist/bin.js' 201: WARN Failed to create bin at /home/runner/work/selenium-ide/selenium-ide/packages/selenium-ide/node_modules/.bin/side-code-export. ENOENT: no such file or directory, open '/home/runner/work/selenium-ide/selenium-ide/packages/selenium-ide/node_modules/side-code-export/dist/bin.js' ... 241: 12:47:46 PM - Project 'packages/side-testkit/tsconfig.json' is out of date because output file 'packages/side-testkit/tsconfig.tsbuildinfo' does not exist 242: 12:47:46 PM - Building project '/home/runner/work/selenium-ide/selenium-ide/packages/side-testkit/tsconfig.json'... 243: 12:47:47 PM - Project 'packages/get-driver/tsconfig.json' is out of date because output file 'packages/get-driver/tsconfig.tsbuildinfo' does not exist 244: 12:47:47 PM - Building project '/home/runner/work/selenium-ide/selenium-ide/packages/get-driver/tsconfig.json'... 245: 12:47:47 PM - Project 'packages/webdriver-testkit/tsconfig.json' is out of date because output file 'packages/webdriver-testkit/tsconfig.tsbuildinfo' does not exist 246: 12:47:47 PM - Building project '/home/runner/work/selenium-ide/selenium-ide/packages/webdriver-testkit/tsconfig.json'... 247: 12:47:47 PM - Project 'packages/side-runtime/tsconfig.json' is out of date because output file 'packages/side-runtime/tsconfig.tsbuildinfo' does not exist 248: 12:47:47 PM - Building project '/home/runner/work/selenium-ide/selenium-ide/packages/side-runtime/tsconfig.json'... 249: ##[error]packages/side-runtime/src/plugins.ts(40,11): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 250: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 251: 12:47:48 PM - Project 'packages/side-code-export/tsconfig.json' can't be built because its dependency 'packages/side-runtime' has errors 252: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-runtime' has errors ... 259: 12:47:48 PM - Project 'packages/code-export-java-junit/tsconfig.json' can't be built because its dependency 'packages/side-code-export' was not built 260: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-java-junit/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export' was not built 261: 12:47:48 PM - Project 'packages/code-export-javascript-mocha/tsconfig.json' can't be built because its dependency 'packages/side-code-export' was not built 262: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-javascript-mocha/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export' was not built 263: 12:47:48 PM - Project 'packages/code-export-python-pytest/tsconfig.json' can't be built because its dependency 'packages/side-code-export' was not built 264: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-python-pytest/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export' was not built 265: 12:47:48 PM - Project 'packages/code-export-ruby-rspec/tsconfig.json' can't be built because its dependency 'packages/side-code-export' was not built 266: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-ruby-rspec/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-code-export' was not built 267: 12:47:48 PM - Project 'packages/side-api/tsconfig.json' can't be built because its dependency 'packages/side-runtime' has errors 268: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/side-api/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-runtime' has errors 269: 12:47:48 PM - Project 'packages/side-example-suite/tsconfig.json' can't be built because its dependency 'packages/code-export-python-pytest' was not built 270: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/side-example-suite/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/code-export-python-pytest' was not built 271: 12:47:48 PM - Project 'packages/side-migrate/tsconfig.json' is out of date because output file 'packages/side-migrate/tsconfig.tsbuildinfo' does not exist 272: 12:47:48 PM - Building project '/home/runner/work/selenium-ide/selenium-ide/packages/side-migrate/tsconfig.json'... 273: ##[error]packages/side-migrate/src/migrations/implicit-locators.ts(25,13): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 274: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 275: ##[error]packages/side-migrate/src/migrations/implicit-locators.ts(27,24): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 276: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 277: ##[error]packages/side-migrate/src/migrations/script-interpolation.ts(25,13): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 278: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 279: ##[error]packages/side-migrate/src/migrations/script-interpolation.ts(27,24): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 280: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 281: ##[error]packages/side-migrate/src/migrations/variable-name.ts(25,13): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 282: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 283: ##[error]packages/side-migrate/src/migrations/variable-name.ts(27,24): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 284: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 285: ##[error]packages/side-migrate/src/legacy/migrate.ts(215,12): error TS7053: Element implicitly has an 'any' type because expression of type 'string' can't be used to index type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 286: No index signature with a parameter of type 'string' was found on type '{ acceptAlert: { name: string; description: string; }; acceptConfirmation: { name: string; description: string; }; addSelection: { name: string; description: string; target: { name: string; description: string; }; }; ... 89 more ...; while: { ...; }; }'. 287: error TS5083: Cannot read file '/home/runner/work/selenium-ide/selenium-ide/packages/side-model-locale-data/tsconfig.json'. 288: 12:47:48 PM - Project 'packages/side-runner/tsconfig.json' can't be built because its dependency 'packages/side-runtime' has errors 289: 12:47:48 PM - Skipping build of project '/home/runner/work/selenium-ide/selenium-ide/packages/side-runner/tsconfig.json' because its dependency '/home/runner/work/selenium-ide/selenium-ide/packages/side-runtime' has errors 290: ERROR: "build:ts" exited with 2. 291: ##[error]Process completed with exit code 1. ```

✨ CI feedback usage guide:

The CI feedback tool (`/checks)` automatically triggers when a PR has a failed check. The tool analyzes the failed checks and provides several feedbacks: - Failed stage - Failed test name - Failure summary - Relevant error logs In addition to being automatically triggered, the tool can also be invoked manually by commenting on a PR: ``` /checks "https://github.com/{repo_name}/actions/runs/{run_number}/job/{job_number}" ``` where `{repo_name}` is the name of the repository, `{run_number}` is the run number of the failed check, and `{job_number}` is the job number of the failed check. #### Configuration options - `enable_auto_checks_feedback` - if set to true, the tool will automatically provide feedback when a check is failed. Default is true. - `excluded_checks_list` - a list of checks to exclude from the feedback, for example: ["check1", "check2"]. Default is an empty list. - `enable_help_text` - if set to true, the tool will provide a help message with the feedback. Default is true. - `persistent_comment` - if set to true, the tool will overwrite a previous checks comment with the new feedback. Default is true. - `final_update_message` - if `persistent_comment` is true and updating a previous checks message, the tool will also create a new message: "Persistent checks updated to latest commit". Default is true. See more information about the `checks` tool in the [docs](https://pr-agent-docs.codium.ai/tools/ci_feedback/).

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Description updated to latest commit (https://github.com/SeleniumHQ/selenium-ide/commit/f825e0b484c82f82f9c064232583fff6a379fd28)

[ ] Copy walkthrough table to "Files Changed" Tab

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Review

⏱️ Estimated effort to review [1-5]	4, because the PR involves significant changes across multiple files, including refactoring and internationalization efforts. The changes affect core functionalities such as command handling and UI text rendering, which require careful review to ensure functionality and consistency across different languages.
🧪 Relevant tests	No
🔍 Possible issues	Possible Bug: The refactoring introduces changes to how commands and language maps are handled. If not properly tested, this could lead to runtime errors or incorrect command executions.
🔍 Possible issues	Localization Issues: The introduction of new language support (Chinese) needs to be thoroughly tested to ensure all text is displayed correctly and that there are no missing translations.
🔒 Security concerns	No

✨ Review tool usage guide:

**Overview:** The `review` tool scans the PR code changes, and generates a PR review which includes several types of feedbacks, such as possible PR issues, security threats and relevant test in the PR. More feedbacks can be [added](https://pr-agent-docs.codium.ai/tools/review/#general-configurations) by configuring the tool. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on any PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L23) related to the review tool (`pr_reviewer` section), use the following template: ``` /review --pr_reviewer.some_config1=... --pr_reviewer.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_reviewer] some_config1=... some_config2=... ``` See the review [usage page](https://pr-agent-docs.codium.ai/tools/review/) for a comprehensive guide on using this tool.

codiumai-pr-agent-pro[bot] commented 2 months ago

PR Code Suggestions

Category	Suggestions
Best practice	Convert the import of `Commands` to a type-only import to prevent runtime imports. ___ Replace the direct import of `Commands` with a type import to ensure that only type information is used during development and no actual code is imported, which aligns with the existing import pattern for `LanguageMap`. [packages/selenium-ide/src/browser/I18N/zh/index.ts [2]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R2-R2) ```diff -import Commands from '@seleniumhq/side-model/dist/I18N/zh/Commands' +import type { Commands } from '@seleniumhq/side-model/dist/I18N/zh/Commands' ```
	Use template literals for multi-line strings to enhance readability. ___ Use template literals for multi-line strings to improve readability and maintainability of the code, especially for helper texts in configuration objects. [packages/selenium-ide/src/browser/I18N/zh/index.ts [65-69]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R65-R69) ```diff -platformUrlHelper: '用例最终展示在此前端页面', -serviceHostHelper: '将用例导出到后端服务的地址', -businessListUrlHelper: '用例所属的业务列表的url地址', +platformUrlHelper: `用例最终展示在此前端页面`, +serviceHostHelper: `将用例导出到后端服务的地址`, +businessListUrlHelper: `用例所属的业务列表的url地址`, ```
	Specify a more precise version range for the `react-intl` dependency. ___ Consider specifying a more precise version range for the `react-intl` dependency to ensure compatibility and prevent automatic updates that might introduce breaking changes. [pnpm-lock.yaml [321]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR321-R321) ```diff react-intl: - specifier: ^6.6.5 + specifier: ~6.6.5 ```
Maintainability	Correct a typo in the variable name for better consistency and readability. ___ Ensure consistency in object property naming by changing `maiMenuChinese` to `mainMenuChinese` to correct the typo and improve code readability. [packages/selenium-ide/src/browser/I18N/zh/index.ts [52]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R52-R52) ```diff -const maiMenuChinese = { tests: '用例', suites: '集合', config: '配置' } +const mainMenuChinese = { tests: '用例', suites: '集合', config: '配置' } ```
	Use enums for repeated string values to centralize control and ease modifications. ___ Introduce enums for repeated string values such as menu names and error messages to centralize control and facilitate changes and translations. [packages/selenium-ide/src/browser/I18N/zh/index.ts [6-11]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R6-R11) ```diff +enum MenuNames { + File = '&文件', + Edit = '&编辑', + View = '&查看', + Help = '&帮助', + Title = '项目编辑器', +} const windowTabChinese = { - file: '&文件', - edit: '&编辑', - view: '&查看', - help: '&帮助', - title: '项目编辑器', + file: MenuNames.File, + edit: MenuNames.Edit, + view: MenuNames.View, + help: MenuNames.Help, + title: MenuNames.Title, } ```
	Remove duplicate imports to clean up the code. ___ Remove the duplicate import statements for `PROJECT_TAB`, `SUITES_TAB`, and `TESTS_TAB` to clean up the code and avoid confusion. [packages/selenium-ide/src/browser/components/AppBar/AppBarTabs.tsx [4-5]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-7aafe3f232281974f4ead5f8994b2bebbaef439651b62ad7bb96342d7c115207R4-R5) ```diff -import { PROJECT_TAB, SUITES_TAB, TESTS_TAB } from 'browser/enums/tab' import { PROJECT_TAB, SUITES_TAB, TESTS_TAB } from 'browser/I18N/keys' ```
	Align the `typescript` version in `peerDependencies` to avoid compatibility issues. ___ Ensure that the version of `typescript` specified in the `peerDependencies` of `@formatjs/intl` is aligned with the versions used elsewhere to avoid potential conflicts or compatibility issues. [pnpm-lock.yaml [2556]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR2556-R2556) ```diff -typescript: ^4.7 \|\| 5 +typescript: ^5.2.2 ```
Enhancement	Refactor menu configurations into a single nested object for improved manageability. ___ Refactor the individual menu configuration objects into a single object with nested properties for better structure and easier management of related configurations. [packages/selenium-ide/src/browser/I18N/zh/index.ts [6-21]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R6-R21) ```diff -const windowTabChinese = { - file: '&文件', - edit: '&编辑', - view: '&查看', - help: '&帮助', - title: '项目编辑器', -} -const electronMenuTreeChinese = { - about: 'Electron信息', - services: '服务信息', - hideElectron: '隐藏Electron', - hideOthers: '隐藏其他', - showAll: '显示所有', - quit: '退出', +const menuConfigChinese = { + windowTab: { + file: '&文件', + edit: '&编辑', + view: '&查看', + help: '&帮助', + title: '项目编辑器', + }, + electronMenu: { + about: 'Electron信息', + services: '服务信息', + hideElectron: '隐藏Electron', + hideOthers: '隐藏其他', + showAll: '显示所有', + quit: '退出', + }, } ```
	Improve type safety and IntelliSense by defining a specific interface for `argTypes`. ___ Consider using a more specific type for the `argTypes` object to ensure type safety and better IntelliSense support in your IDE. You can define a specific interface for `argTypes` that includes all the properties you expect, rather than using the generic `ArgTypes` type. [packages/side-model/src/I18N/zh/ArgTypes.ts [6]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-08af6e86a51944378e91d86eb1be63715df39fe50a829a3d3e48d49e4de75891R6-R6) ```diff -export const argTypes: ArgTypes = { +interface SpecificArgTypes { + alertText: { name: string; description: string; }; + answer: { name: string; description: string; }; + attributeLocator: { name: string; description: string; }; + // Add other properties as needed +} +export const argTypes: SpecificArgTypes = { ```
	Pass a dynamic locale to `IntlProvider` for internationalization. ___ Ensure that the `IntlProvider` component has a `locale` prop passed to it, which should dynamically set the locale based on user preference or system settings. [packages/selenium-ide/src/browser/components/AppWrapper.tsx [35]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-49a71163c7c32c0a9b72299ebb6b1521884dea7debccb2cf0c0229b2d8e86965R35-R35) ```diff - + ```
	Update `hoist-non-react-statics` to the latest version. ___ Update the `hoist-non-react-statics` package to a newer version if available to take advantage of bug fixes and new features. [pnpm-lock.yaml [3540]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR3540-R3540) ```diff -hoist-non-react-statics: 3.3.2 +hoist-non-react-statics: 3.3.3 # Assuming 3.3.3 is the latest version ```
Possible issue	Add conditional rendering to handle potential null or undefined `languageMap`. ___ Use a conditional rendering approach to handle cases where `languageMap` might not be loaded yet, to prevent runtime errors in your React component. [packages/selenium-ide/src/browser/windows/Splash/renderer.tsx [36]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-e0e590a36a80dfa8b99079668c34ac44fff11491f19b08e06c219f7ab22a348fR36-R36) ```diff - +{languageMap.splash ? : Loading...} ```
Performance	Move the loading URL check to the top of the component to improve performance and readability. ___ Move the conditional rendering check for `url === 'http://loading'` to the beginning of the component function to avoid unnecessary hook calls and state updates when the URL is loading. [packages/selenium-ide/src/browser/windows/ProjectEditor/tabs/Project/ProjectSettings.tsx [25]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-95c62b6c1509005f6c74c7a0f61d39935518d6ca9478362de2486eb47f83bbbaR25-R25) ```diff +if (url === 'http://loading') { + return null; +} const [languageMap, setLanguageMap] = useState({ ```
Performance	Remove redundant `tslib` dependency where not specifically needed. ___ Consider removing the redundant dependency `tslib` from multiple packages if it is globally available or not specifically needed by each package, to simplify the dependency tree and potentially improve install times. [pnpm-lock.yaml [8875]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR8875-R8875) ```diff -tslib: 2.6.0 +# Removed redundant tslib dependency ```
Security	Ensure the integrity hashes for new packages are correct to enhance security. ___ Verify and ensure that the integrity hashes for all newly added packages are correct to prevent potential security risks from tampered packages. [pnpm-lock.yaml [7491]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR7491-R7491) ```diff -resolution: {integrity: sha512-eYq5fkFBVxc7GIFDzpFQkDOZgNayNTQn4Oufe8jw6YY6OHVw70/4pA3FyCsQ0Gb2DnvEJEMmN2tOaXUGByM+kg==} +resolution: {integrity: sha512-verifiedCorrectHash==} ```

✨ Improve tool usage guide:

**Overview:** The `improve` tool scans the PR code changes, and automatically generates suggestions for improving the PR code. The tool can be triggered [automatically](https://pr-agent-docs.codium.ai/usage-guide/automations_and_usage/#github-app-automatic-tools-when-a-new-pr-is-opened) every time a new PR is opened, or can be invoked manually by commenting on a PR. - When commenting, to edit [configurations](https://github.com/Codium-ai/pr-agent/blob/main/pr_agent/settings/configuration.toml#L78) related to the improve tool (`pr_code_suggestions` section), use the following template: ``` /improve --pr_code_suggestions.some_config1=... --pr_code_suggestions.some_config2=... ``` - With a [configuration file](https://pr-agent-docs.codium.ai/usage-guide/configuration_options/), use the following template: ``` [pr_code_suggestions] some_config1=... some_config2=... ``` See the improve [usage page](https://pr-agent-docs.codium.ai/tools/improve/) for a comprehensive guide on using this tool.

fernandozw commented 2 months ago

@toddtarsi Your idea is right, using react-intl for translation eliminates the need for developers to maintain it themselves, which is great!

toddtarsi commented 2 months ago

@fernandozw - I think it's good here, and I'm going to merge to the latest on trunk. What I'd really appreciate from your side is to test the latest on trunk in the next few days, and if it looks good as a release candidate, we'll send it out! I figure by merging it to trunk, even if it's 90% right and 10% wrong, it will be an easier workflow for you to do fixes there before we increment and truly release, even if the latest tag is updated.

Category	Suggestions
Best practice	Convert the import of `Commands` to a type-only import to prevent runtime imports. ___ Replace the direct import of `Commands` with a type import to ensure that only type information is used during development and no actual code is imported, which aligns with the existing import pattern for `LanguageMap`. [packages/selenium-ide/src/browser/I18N/zh/index.ts [2]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R2-R2) ```diff -import Commands from '@seleniumhq/side-model/dist/I18N/zh/Commands' +import type { Commands } from '@seleniumhq/side-model/dist/I18N/zh/Commands' ```
	Use template literals for multi-line strings to enhance readability. ___ Use template literals for multi-line strings to improve readability and maintainability of the code, especially for helper texts in configuration objects. [packages/selenium-ide/src/browser/I18N/zh/index.ts [65-69]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R65-R69) ```diff -platformUrlHelper: '用例最终展示在此前端页面', -serviceHostHelper: '将用例导出到后端服务的地址', -businessListUrlHelper: '用例所属的业务列表的url地址', +platformUrlHelper: `用例最终展示在此前端页面`, +serviceHostHelper: `将用例导出到后端服务的地址`, +businessListUrlHelper: `用例所属的业务列表的url地址`, ```
	Specify a more precise version range for the `react-intl` dependency. ___ Consider specifying a more precise version range for the `react-intl` dependency to ensure compatibility and prevent automatic updates that might introduce breaking changes. [pnpm-lock.yaml [321]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR321-R321) ```diff react-intl: - specifier: ^6.6.5 + specifier: ~6.6.5 ```
Maintainability	Correct a typo in the variable name for better consistency and readability. ___ Ensure consistency in object property naming by changing `maiMenuChinese` to `mainMenuChinese` to correct the typo and improve code readability. [packages/selenium-ide/src/browser/I18N/zh/index.ts [52]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R52-R52) ```diff -const maiMenuChinese = { tests: '用例', suites: '集合', config: '配置' } +const mainMenuChinese = { tests: '用例', suites: '集合', config: '配置' } ```
	Use enums for repeated string values to centralize control and ease modifications. ___ Introduce enums for repeated string values such as menu names and error messages to centralize control and facilitate changes and translations. [packages/selenium-ide/src/browser/I18N/zh/index.ts [6-11]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R6-R11) ```diff +enum MenuNames { + File = '&文件', + Edit = '&编辑', + View = '&查看', + Help = '&帮助', + Title = '项目编辑器', +} const windowTabChinese = { - file: '&文件', - edit: '&编辑', - view: '&查看', - help: '&帮助', - title: '项目编辑器', + file: MenuNames.File, + edit: MenuNames.Edit, + view: MenuNames.View, + help: MenuNames.Help, + title: MenuNames.Title, } ```
	Remove duplicate imports to clean up the code. ___ Remove the duplicate import statements for `PROJECT_TAB`, `SUITES_TAB`, and `TESTS_TAB` to clean up the code and avoid confusion. [packages/selenium-ide/src/browser/components/AppBar/AppBarTabs.tsx [4-5]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-7aafe3f232281974f4ead5f8994b2bebbaef439651b62ad7bb96342d7c115207R4-R5) ```diff -import { PROJECT_TAB, SUITES_TAB, TESTS_TAB } from 'browser/enums/tab' import { PROJECT_TAB, SUITES_TAB, TESTS_TAB } from 'browser/I18N/keys' ```
	Align the `typescript` version in `peerDependencies` to avoid compatibility issues. ___ Ensure that the version of `typescript` specified in the `peerDependencies` of `@formatjs/intl` is aligned with the versions used elsewhere to avoid potential conflicts or compatibility issues. [pnpm-lock.yaml [2556]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR2556-R2556) ```diff -typescript: ^4.7 \|\| 5 +typescript: ^5.2.2 ```
Enhancement	Refactor menu configurations into a single nested object for improved manageability. ___ Refactor the individual menu configuration objects into a single object with nested properties for better structure and easier management of related configurations. [packages/selenium-ide/src/browser/I18N/zh/index.ts [6-21]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-252ec41818e7e19f57caddce35b06b2d0757ff585a656bbb4b2148cbdd697a52R6-R21) ```diff -const windowTabChinese = { - file: '&文件', - edit: '&编辑', - view: '&查看', - help: '&帮助', - title: '项目编辑器', -} -const electronMenuTreeChinese = { - about: 'Electron信息', - services: '服务信息', - hideElectron: '隐藏Electron', - hideOthers: '隐藏其他', - showAll: '显示所有', - quit: '退出', +const menuConfigChinese = { + windowTab: { + file: '&文件', + edit: '&编辑', + view: '&查看', + help: '&帮助', + title: '项目编辑器', + }, + electronMenu: { + about: 'Electron信息', + services: '服务信息', + hideElectron: '隐藏Electron', + hideOthers: '隐藏其他', + showAll: '显示所有', + quit: '退出', + }, } ```
	Improve type safety and IntelliSense by defining a specific interface for `argTypes`. ___ Consider using a more specific type for the `argTypes` object to ensure type safety and better IntelliSense support in your IDE. You can define a specific interface for `argTypes` that includes all the properties you expect, rather than using the generic `ArgTypes` type. [packages/side-model/src/I18N/zh/ArgTypes.ts [6]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-08af6e86a51944378e91d86eb1be63715df39fe50a829a3d3e48d49e4de75891R6-R6) ```diff -export const argTypes: ArgTypes = { +interface SpecificArgTypes { + alertText: { name: string; description: string; }; + answer: { name: string; description: string; }; + attributeLocator: { name: string; description: string; }; + // Add other properties as needed +} +export const argTypes: SpecificArgTypes = { ```
	Pass a dynamic locale to `IntlProvider` for internationalization. ___ Ensure that the `IntlProvider` component has a `locale` prop passed to it, which should dynamically set the locale based on user preference or system settings. [packages/selenium-ide/src/browser/components/AppWrapper.tsx [35]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-49a71163c7c32c0a9b72299ebb6b1521884dea7debccb2cf0c0229b2d8e86965R35-R35) ```diff - + ```
	Update `hoist-non-react-statics` to the latest version. ___ Update the `hoist-non-react-statics` package to a newer version if available to take advantage of bug fixes and new features. [pnpm-lock.yaml [3540]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR3540-R3540) ```diff -hoist-non-react-statics: 3.3.2 +hoist-non-react-statics: 3.3.3 # Assuming 3.3.3 is the latest version ```
Possible issue	Add conditional rendering to handle potential null or undefined `languageMap`. ___ Use a conditional rendering approach to handle cases where `languageMap` might not be loaded yet, to prevent runtime errors in your React component. [packages/selenium-ide/src/browser/windows/Splash/renderer.tsx [36]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-e0e590a36a80dfa8b99079668c34ac44fff11491f19b08e06c219f7ab22a348fR36-R36) ```diff - +{languageMap.splash ? : Loading...} ```
Performance	Move the loading URL check to the top of the component to improve performance and readability. ___ Move the conditional rendering check for `url === 'http://loading'` to the beginning of the component function to avoid unnecessary hook calls and state updates when the URL is loading. [packages/selenium-ide/src/browser/windows/ProjectEditor/tabs/Project/ProjectSettings.tsx [25]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-95c62b6c1509005f6c74c7a0f61d39935518d6ca9478362de2486eb47f83bbbaR25-R25) ```diff +if (url === 'http://loading') { + return null; +} const [languageMap, setLanguageMap] = useState({ ```
Performance	Remove redundant `tslib` dependency where not specifically needed. ___ Consider removing the redundant dependency `tslib` from multiple packages if it is globally available or not specifically needed by each package, to simplify the dependency tree and potentially improve install times. [pnpm-lock.yaml [8875]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR8875-R8875) ```diff -tslib: 2.6.0 +# Removed redundant tslib dependency ```
Security	Ensure the integrity hashes for new packages are correct to enhance security. ___ Verify and ensure that the integrity hashes for all newly added packages are correct to prevent potential security risks from tampered packages. [pnpm-lock.yaml [7491]](https://github.com/SeleniumHQ/selenium-ide/pull/1820/files#diff-32824c984905bb02bc7ffcef96a77addd1f1602cff71a11fbbfdd7f53ee026bbR7491-R7491) ```diff -resolution: {integrity: sha512-eYq5fkFBVxc7GIFDzpFQkDOZgNayNTQn4Oufe8jw6YY6OHVw70/4pA3FyCsQ0Gb2DnvEJEMmN2tOaXUGByM+kg==} +resolution: {integrity: sha512-verifiedCorrectHash==} ```

SeleniumHQ / selenium-ide