Fix: UI Fixes on Infra Page #2221
Conversation
WalkthroughThe update modifies the display logic for CPU core counts and temperature units in the infrastructure monitoring status boxes. It now conditionally uses "core" or "cores" based on count and updates the temperature unit from "C" to "°C" for greater precision. No other logic or control flow was changed. Changes
Assessment against linked issues
Why did the Canadian computer get so good at infrastructure monitoring? Tip ⚡️ Faster reviews with caching
Enjoy the performance boost—your workflow just got faster. ✨ 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 (
|
![llamapreview[bot]](https://avatars.githubusercontent.com/in/1023541?s=60&v=4){kind=small}
There was a problem hiding this comment.
Auto Pull Request Review from LlamaPReview
Review Status: Automated Review Skipped
Dear contributor,
Thank you for your Pull Request. LlamaPReview has analyzed your changes and determined that this PR does not require an automated code review.
Analysis Result:
The PR contains only minor UI fixes, specifically text label improvements and unit display updates. These changes are purely stylistic and do not impact core logic, business rules, or security. The changes are well-contained within a single component and do not affect cross-module dependencies or performance. Therefore, no automated code review is required.
We're continuously improving our PR analysis capabilities. Have thoughts on when and how LlamaPReview should perform automated reviews? Share your insights in our GitHub Discussions.
Best regards,
LlamaPReview Team
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
| {cpuTemperature.toFixed(2)} | ||
| <Typography component="span">C</Typography> | ||
| <Typography component="span">°C</Typography> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Localize the temperature unit symbol
Switching to "°C" is a great UI tweak—just wrap it in t so translators can adjust or replace it (sorry Americans, Canadians will crack a smile with real Celsius notation, eh?):
- <Typography component="span">°C</Typography>
+ <Typography component="span">{t("unitCelsius")}</Typography>Add a unitCelsius key in your translation files.
| <Typography component="span"> | ||
| {logicalCores === 1 ? "core" : "cores"} | ||
| </Typography> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Apply the same i18n pattern to logical cores
Ditto for logical cores—keep it consistent and translatable:
- <Typography component="span">
- {logicalCores === 1 ? "core" : "cores"}
- </Typography>
+ <Typography component="span">
+ {t("core", { count: logicalCores })}
+ </Typography>This ensures both physical and logical counts are localized correctly.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography component="span"> | |
| {logicalCores === 1 ? "core" : "cores"} | |
| </Typography> | |
| </> | |
| <Typography component="span"> | |
| {t("core", { count: logicalCores })} | |
| </Typography> |
| <Typography component="span"> | ||
| {physicalCores === 1 ? "core" : "cores"} | ||
| </Typography> | ||
| </> |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure i18n for singular/plural CPU core labels
The hardcoded "core"/"cores" strings won’t be picked up by your translation framework. Leverage t with count-based pluralization so translators (and our Canadian friends) can handle it properly:
- <Typography component="span">
- {physicalCores === 1 ? "core" : "cores"}
- </Typography>
+ <Typography component="span">
+ {t("core", { count: physicalCores })}
+ </Typography>Make sure the "core" key is defined in your locales with singular/plural forms.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| <Typography component="span"> | |
| {physicalCores === 1 ? "core" : "cores"} | |
| </Typography> | |
| </> | |
| <> | |
| <Typography component="span"> | |
| {t("core", { count: physicalCores })} | |
| </Typography> | |
| </> |
3 participants
Describe your changes
This PR address minor UI fixes in infrastructure page.
Fixes #2168
Please ensure all items are checked off before requesting a review. "Checked off" means you need to add an "x" character between brackets so they turn into checkmarks.
<div>Add</div>, use):Summary by CodeRabbit