fix(hubble): harden frontend validation and update license dependencies#741
Conversation
There was a problem hiding this comment.
Pull request overview
This PR strengthens Hubble frontend UX and validation by aligning “favorite name” rules with backend-compatible constraints (including i18n messaging), improving navigation feedback when operations are disabled, and updating the distro’s known-dependencies list.
Changes:
- Added favorite-name validation helpers/rules (type guard + regex) with en/zh i18n messages, and updated tests accordingly.
- Enhanced navigation item rendering to surface aggregated disable reasons, with a ConsoleItem test to cover the “Dashboard unavailable” scenario.
- Updated dependency allowlist to include
kotlin-stdlib-common-1.6.20.jar.
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| hugegraph-hubble/hubble-fe/src/utils/rules.js | Adds isValidFavoriteName + isFavoriteName; switches account-name validation to reject Error objects. |
| hugegraph-hubble/hubble-fe/src/utils/rules.test.js | Extends i18n rule tests with favorite-name coverage. |
| hugegraph-hubble/hubble-fe/src/modules/analysis/QueryBar/ContentCommon/index.js | Disables/enables favorite submission based on backend-compatible favorite names and shows input error status. |
| hugegraph-hubble/hubble-fe/src/modules/analysis/QueryBar/ContentCommon/index.test.js | New test ensuring favorite submission remains disabled until the name is valid. |
| hugegraph-hubble/hubble-fe/src/modules/navigation/Item/index.js | Displays aggregated disabled reasons below the item list for better UX. |
| hugegraph-hubble/hubble-fe/src/modules/navigation/Item/index.module.scss | Adds styling for the disabled-reason text block. |
| hugegraph-hubble/hubble-fe/src/modules/navigation/ConsoleItem/index.test.js | Adds coverage for showing the “Dashboard unavailable” reason. |
| hugegraph-hubble/hubble-fe/src/i18n/resources/en-US/components/common.json | Adds favorite_name_rule translation. |
| hugegraph-hubble/hubble-fe/src/i18n/resources/zh-CN/components/common.json | Adds favorite_name_rule translation. |
| hugegraph-dist/scripts/dependency/known-dependencies.txt | Adds kotlin-stdlib-common-1.6.20.jar to known dependencies. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🚨 Review summary
Important
The release license inventory is incomplete, favorite-name validation remains inconsistent across entry points, and the new disabled-state explanation can overflow its navigation container.
🧪 How this was verified
| Independent check | What it traced | Outcome |
|---|---|---|
| 🧾 License inventory | Followed check_dependencies.sh, known-dependencies.txt, hugegraph-dist/pom.xml, and the release-docs/LICENSE copied into the archive. |
Confirmed that the checker accepts kotlin-stdlib-common-1.6.20.jar while the packaged LICENSE still declares only 1.5.31. |
| ✍️ Favorite-name contract | Exercised invalid names against ContentCommon, traced isValidFavoriteName/isFavoriteName, and compared the analysis and algorithm create/rename flows that call the same collection API. |
Retained two findings: four sibling flows still submit backend-invalid names, and the query-bar flow blocks input without displaying the new localized rule. |
| 🧭 Navigation layout | Enumerated Item callers and compared the appended 160px reason with the 230px fixed parent and existing button/header dimensions. |
Retained the overflow/layout finding; the separate contrast suggestion was dropped as lower-signal after lead verification. |
Note
Targeted frontend tests could not run in the review checkout because react-scripts was not installed. The retained findings were therefore verified through exact call-site, packaging, and layout traces rather than claimed as test-proven.
🤖 Codex review · GPT-5.6 Sol · effort: xhigh
- align favorite-name validation across all editors - show localized validation guidance for invalid names - allow navigation rows to grow for disabled reasons - add Kotlin dependency to the packaged license - cover all favorite creation and rename flows
There was a problem hiding this comment.
🚨 Review summary
Important
The validation and license changes align with their contracts, but the disabled-state explanation does not provide accessible text contrast.
📊 Risk dashboard
| Signal | Result |
|---|---|
| 🚦 Review gate | Blocked |
| 🔎 Actionable findings | 1 (1 P2) |
| 🧪 Verification coverage | 4 checks |
| 1 / 0 |
🔬 Coverage details
⚠️ Navigation accessibility — Confirmed
Trace: Item, ConsoleItem, Home, navigation sizing and disabled-state styles
Conclusion: The reviewer retained the contrast defect after measuring the changed reason text and dropped the separate layout-shift concern as non-actionable.
✅ Favorite-name validation contract — Clear
Trace: FavoriteNameInput, isValidFavoriteName, QueryBar, all log create and rename flows, Constant.COMMON_NAME_PATTERN
Conclusion: The reviewer confirmed that the frontend type, character-set, and 48-character boundary checks match the backend naming contract, so no validation finding survived.
✅ Dependency and license inventory — Clear
Trace: dependency generation scripts, known-dependencies allowlist, packaged LICENSE
Conclusion: The reviewer confirmed that both generated inventories contain kotlin-stdlib-common 1.6.20 and found no release or compliance regression.
✅ Current-head consistency checks — Clear
Trace: scripts/check-i18n.js, git diff --check, current GitHub status checks
Conclusion: The i18n check reported 1,639 keys per locale, git diff validation was clean, and every current GitHub check completed successfully.
Warning
Verification limits
- Targeted Jest tests could not run because react-scripts is not installed in the checkout.
🤖 Codex review · GPT-5.6 Sol · effort: xhigh
|
|
||
| .reason { | ||
| width: 160px; | ||
| color: #8c8c8c; |
There was a problem hiding this comment.
⚠️ P2 · Increase the disabled-reason text contrast
📐 Measured result
| Metric | Observed | Required / target |
|---|---|---|
| Text contrast | 3.36:1 | >= 4.5:1 |
The changed 12px reason text uses #8c8c8c over the white navigation background, producing a 3.36:1 contrast ratio.
References: WCAG 2.2 contrast minimum
Warning
Impact: Low-vision users may be unable to read the persistent explanation of why Dashboard operations are disabled.
🛠️ Suggested change
Use a darker text color that reaches at least 4.5:1, such as #595959, which provides approximately 7.00:1 contrast.
🤖 Codex review · GPT-5.6 Sol · effort: xhigh
This PR contains the following changes:
Please review and merge, thanks.