Microcopy updates for "project-scoped" items #4448
Conversation
WalkthroughThis update adds periods to the end of several helper texts and descriptions across multiple frontend components for punctuation consistency. It also replaces PatternFly list components with nested Content components using HTML list semantics in a hardware profile form section and refines a label text for clarity without altering any logic or behavior. Additionally, related test strings were updated to reflect the label change. Changes
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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
CodeRabbit Configuration File (
|
|
[APPROVALNOTIFIER] This PR is NOT APPROVED This pull-request has been approved by: The full list of commands accepted by this bot can be found here.
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/src/pages/pipelines/global/modelCustomization/trainingHardwareSection/TrainingHardwareProfileFormSection.tsx (1)
82-107: Replace inline spacing with PatternFly utility classesInline styles (
style={{ marginTop: 10, marginBottom: 8 }}andstyle={{ marginTop: 8, marginBottom: 10 }}) work but bypass PatternFly’s spacing utilities, making future style maintenance harder and risking inconsistency across themes.- <> - <div style={{ marginTop: 10, marginBottom: 8 }} /> + <> + <div className="pf-v5-u-my-sm" /> ... - <div style={{ marginTop: 8, marginBottom: 10 }} /> + <div className="pf-v5-u-my-sm" />Using
pf-v5-u-my-sm(or another appropriate utility) delegates spacing to the design system and avoids hard-coded pixel values.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
frontend/src/components/ProjectScopedPopover.tsx(1 hunks)frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx(1 hunks)frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx(1 hunks)frontend/src/pages/pipelines/global/modelCustomization/trainingHardwareSection/TrainingHardwareProfileFormSection.tsx(2 hunks)
🔇 Additional comments (3)
frontend/src/pages/modelServing/screens/projects/InferenceServiceModal/ManageInferenceServiceModal.tsx (1)
201-204: Helper-text punctuation change looks goodThe period adds the missing punctuation and keeps micro-copy consistent with the rest of the UI.
frontend/src/components/ProjectScopedPopover.tsx (1)
22-26: Micro-copy consistency update approvedBoth sentences now end with a period, matching the style guide used elsewhere.
frontend/src/pages/modelServing/screens/projects/kServeModal/ManageKServeModal.tsx (1)
381-387: Minor punctuation fix acknowledgedThe trailing period completes the sentence; no functional impact.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4448 +/- ##
==========================================
- Coverage 82.61% 82.52% -0.10%
==========================================
Files 1756 1761 +5
Lines 36696 36853 +157
Branches 10859 10924 +65
==========================================
+ Hits 30318 30413 +95
- Misses 6378 6440 +62
... and 58 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
| projectScopedHardwareProfiles['1'] && | ||
| projectScopedHardwareProfiles['0'].length > 0 && ( | ||
| <> | ||
| <div style={{ marginTop: 10, marginBottom: 8 }} /> |
There was a problem hiding this comment.
We should avoid using these hardcoded value and the custom styles if possible. When meeting these layout issues, check the PatternFly doc to see if there are existing solutions. Here are some solutions you can consider
- Replace
<List>and<ListItem>with the<Content>as the example shows, that could generate a proper space between the element above and below - Consider using Stack layout here. This could ensure every element in it has some specific spaces
I'd recommend the first solution because we should try to follow what PatternFly designs as much as possible, that could make sure the layout to follow their design guidelines and standard.
| {isProjectScoped && | ||
| projectScopedHardwareProfiles['1'] && | ||
| projectScopedHardwareProfiles['0'].length > 0 && ( | ||
| <Content> |
There was a problem hiding this comment.
Use <Content component={ContentVariants.ul}> and <Content component={ContentVariants.li}> to replace ul and li here, avoid using the native HTML element.
There was a problem hiding this comment.
This Content will not be needed anymore
For: https://issues.redhat.com/browse/RHOAIENG-27526
Description
Used this doc and make updates accordingly. The main changes includes
How Has This Been Tested?
Tested the change by navigating to the specific section of the website where the update was made and confirming that the expected change appeared as intended.
Test Impact
Request review criteria:
Self checklist (all need to be checked):
If you have UI changes:
After the PR is posted & before it merges:
mainSummary by CodeRabbit
Summary by CodeRabbit