-
Notifications
You must be signed in to change notification settings - Fork 50
🐛 Fix missing tooltip text in create platform wizard #2604
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Signed-off-by: Maayan Hadasi <[email protected]>
WalkthroughAdds an English translation key for a platform URL tooltip, changes tooltip fallback to use that translation, and updates the platform form to watch the selected kind and compute the URL tooltip dynamically based on current selection. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant PF as PlatformForm (React)
participant HF as useForm/watch
participant KL as usePlatformKindList.getUrlTooltip
participant I18N as i18n t()
U->>PF: Open Create/Edit Platform
PF->>HF: watch("kind")
HF-->>PF: selectedKind updates
PF->>KL: getUrlTooltip(selectedKind)
alt Known kind
KL-->>PF: Kind-specific tooltip
else Unknown kind
KL->>I18N: t("tooltip.platformUrl")
I18N-->>KL: "API URL to the source platform"
KL-->>PF: Fallback tooltip
end
PF-->>U: Show URL field with tooltip
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested reviewers
Pre-merge checks (3 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (3)
client/public/locales/en/translation.json (1)
619-621
: Polish tooltip copy and punctuationConsider adding a terminal period for consistency with nearby tooltips (and optionally “of” instead of “to”).
- "platformUrl": "API URL to the source platform" + "platformUrl": "API URL to the source platform."client/src/app/pages/source-platforms/usePlatformKindList.ts (1)
41-49
: Good fix: translated fallback + correct memo depsReturning
t("tooltip.platformUrl")
for unknown kinds and adding[t]
to the callback deps addresses the missing tooltip in the create wizard and avoids stale translations.If you plan to support multiple kinds, consider i18n-izing
KIND_MAP
strings as well to avoid mixed localized and hardcoded tooltips.client/src/app/pages/source-platforms/components/platform-form.tsx (1)
238-253
: Optional: sort identity options for UXSorting
identitiesOptions
alphabetically improves findability in the typeahead.- const identitiesOptions = identities - .filter(({ kind }) => kind === "source") - .map((identity) => identity.name); + const identitiesOptions = identities + .filter(({ kind }) => kind === "source") + .map((identity) => identity.name) + .sort((a, b) => a.localeCompare(b));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
client/public/locales/en/translation.json
(1 hunks)client/src/app/pages/source-platforms/components/platform-form.tsx
(5 hunks)client/src/app/pages/source-platforms/usePlatformKindList.ts
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-08-29T16:01:04.207Z
Learnt from: sjd78
PR: konveyor/tackle2-ui#2583
File: client/src/app/pages/identities/components/identity-form/kind-source-form.tsx:17-26
Timestamp: 2025-08-29T16:01:04.207Z
Learning: Skip i18n/internationalization suggestions for the file client/src/app/pages/identities/components/identity-form/kind-source-form.tsx - hardcoded strings like "Username/Password" and "Private Key/Passphrase" should remain as-is.
Applied to files:
client/src/app/pages/source-platforms/components/platform-form.tsx
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: unit-test
- GitHub Check: build-and-upload-for-global-ci
🔇 Additional comments (4)
client/src/app/pages/source-platforms/usePlatformKindList.ts (1)
4-5
: Imports OKAdded types are appropriate.
client/src/app/pages/source-platforms/components/platform-form.tsx (3)
117-122
: Primary fix looks good: dynamic URL tooltip based on current selectionUsing
watch("kind")
and passingselectedKind
intogetUrlTooltip
correctly updates the tooltip when the user changes the kind in the create wizard.Also applies to: 142-143, 217-217
2-2
: Import reorg/additions acknowledgedLooks fine.
Also applies to: 5-5, 16-20, 25-28, 31-31, 34-34
72-76
: Preserve selected identity on edit
- Destructure
setValue
fromuseForm
(no need to defaultidentities
—useFetchIdentities()
already returns[]
).- After
selectedKind
, add:React.useEffect(() => { if (platform?.identity?.id && identities.length) { const name = identities.find(i => i.id === platform.identity.id)?.name; if (name) setValue("credentials", name, { shouldDirty: false }); } }, [identities, platform?.identity?.id, setValue]);- Confirm whether
updatePlatform
uses PUT (which may clear omitted fields) or PATCH (which does not) to avoid accidentally unsetting the identity.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is ok.
I added useRepositoryKind() that uses a different approach for static list with translated keys. It would be good to harmonize around a common approach if/when we have to update again, or if there is another hook that provides a set of static data.
Resolves: https://issues.redhat.com/browse/MTA-6124
Resolves: #2602
Screencast_20250910_153810.webm
Summary by CodeRabbit
New Features
Chores