-
Notifications
You must be signed in to change notification settings - Fork 26
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
fix(ui): small fixes #1505
base: main
Are you sure you want to change the base?
fix(ui): small fixes #1505
Conversation
📝 WalkthroughWalkthroughThis update adds a new error message key to multiple locale JSON configuration files, ensuring that the stats server port field cannot be left empty. Additionally, styling adjustments have been made to several components: the IconButton now prevents shrinking and disables pointer events in its disabled state; MonerodMarkup introduces a new AddButtonContainer for node additions and disables node removal when only one exists; and P2poolMarkup centralizes error validation and messaging. Minor styling enhancements and an i18n configuration update further refine the user interface and experience. Changes
Sequence Diagram(s)sequenceDiagram
participant U as User
participant M as MonerodMarkup Component
participant NS as Node State
participant UI as User Interface
U->>M: Clicks "Add" button
M->>NS: Append new node entry
NS-->>M: Return updated nodes list
M->>UI: Re-render component with new node
U->>M: Attempt to remove node (button disabled if only one node)
sequenceDiagram
participant U as User
participant P as P2poolMarkup Component
participant V as Validation Logic
participant UI as User Interface
U->>P: Enters stats server port (empty)
P->>V: Validate input
V-->>P: Returns error (empty input)
P->>UI: Set and display errorMessage ("Stats server port cannot be empty")
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: .coderabbit.yml 📒 Files selected for processing (12)
🚧 Files skipped from review as they are similar to previous changes (12)
⏰ Context from checks skipped due to timeout of 90000ms (3)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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
Documentation and Community
|
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: 7
🧹 Nitpick comments (7)
src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.tsx (1)
29-29
: Consider strengthening the URL validation regex.The current regex might allow some invalid URLs. Consider using a more restrictive pattern that validates the protocol and port range.
-const node_url_regex = /^(https?:\/\/[a-zA-Z0-9.-]+(:\d{1,5})?)(\/.*)?$/; +const node_url_regex = /^https?:\/\/[a-zA-Z0-9][-a-zA-Z0-9.]*[a-zA-Z0-9](:\d{1,5})?(?:\/[-a-zA-Z0-9._~:/?#[\]@!$&'()*+,;=]*)?$/;src/containers/floating/Settings/sections/experimental/P2poolMarkup.tsx (1)
73-76
: Consider improving the port fallback logic.Using 0 as a fallback for
currentStatsServerInputPort
might mask potential issues. Consider usingundefined
ornull
to make the unset state more explicit.- () => (isRandomStatsServerPort ? currentStatsServerPort : editedCustomStatsServerPort) || 0, + () => (isRandomStatsServerPort ? currentStatsServerPort : editedCustomStatsServerPort) ?? null,public/locales/cn/settings.json (1)
54-54
: Translate the error message to Chinese.The error message is currently in English. For consistency with other messages in the Chinese locale file, it should be translated to Chinese.
Apply this diff to translate the error message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty", + "invalid-stats-server-port-empty": "统计服务器端口不能为空",public/locales/ko/settings.json (1)
54-54
: Translate the error message to Korean.The error message is currently in English. For consistency with other messages in the Korean locale file, it should be translated to Korean.
Apply this diff to translate the error message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty", + "invalid-stats-server-port-empty": "통계 서버 포트를 비워둘 수 없습니다",public/locales/ja/settings.json (1)
54-54
: Translate the error message to Japanese.The error message is currently in English. For consistency with other messages in the Japanese locale file, it should be translated to Japanese.
Apply this diff to translate the error message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty", + "invalid-stats-server-port-empty": "統計サーバーポートを空にすることはできません",public/locales/hi/settings.json (1)
54-54
: Translate the error message to Hindi.The error message is currently in English. For consistency with other messages in the Hindi locale file, it should be translated to Hindi.
Apply this diff to translate the error message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty", + "invalid-stats-server-port-empty": "सांख्यिकी सर्वर पोर्ट खाली नहीं हो सकता",public/locales/pl/settings.json (1)
15-15
: Remove redundant null entries.The file contains two null entries that appear to be redundant or deprecated.
Apply this diff to remove the null entries:
- "connected-to-tari-network": null, - "not-connected-to-tari-network": null,Also applies to: 75-75
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (19)
public/locales/af/settings.json
(1 hunks)public/locales/cn/settings.json
(1 hunks)public/locales/de/settings.json
(1 hunks)public/locales/en/settings.json
(1 hunks)public/locales/fr/settings.json
(1 hunks)public/locales/hi/settings.json
(1 hunks)public/locales/id/settings.json
(1 hunks)public/locales/ja/settings.json
(1 hunks)public/locales/ko/settings.json
(1 hunks)public/locales/pl/settings.json
(1 hunks)public/locales/ru/settings.json
(1 hunks)public/locales/tr/settings.json
(1 hunks)src/components/elements/buttons/IconButton.tsx
(2 hunks)src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.styles.ts
(2 hunks)src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.tsx
(3 hunks)src/containers/floating/Settings/sections/experimental/P2poolMarkup.tsx
(3 hunks)src/containers/main/Dashboard/MiningView/components/BlockTime.styles.ts
(2 hunks)src/containers/main/Dashboard/MiningView/components/Ruler.styles.ts
(0 hunks)src/theme/styles.ts
(1 hunks)
💤 Files with no reviewable changes (1)
- src/containers/main/Dashboard/MiningView/components/Ruler.styles.ts
🔇 Additional comments (9)
src/containers/main/Dashboard/MiningView/components/BlockTime.styles.ts (2)
10-21
: LGTM! Gradient implementation effectively addresses ruler mark cut-offs.The gradient overlay implementation in
BlockTimeContainer
aligns well with the PR objectives, providing a smooth transition that eliminates harsh cut-offs. The z-index layering ensures proper stacking of elements.
27-27
: LGTM! Proper z-index handling for text elements.The z-index additions to typography components ensure text remains visible above the gradient overlay.
Also applies to: 39-39
src/theme/styles.ts (1)
14-23
: LGTM! Consistent gradient implementation across components.The radial gradient implementation in
DashboardContainer
maintains visual consistency with the BlockTime component while providing a broader, more subtle effect appropriate for the dashboard container. The z-index layering ensures proper integration with existing visual elements.src/components/elements/buttons/IconButton.tsx (1)
15-15
: Great improvements to button behavior!The addition of
flex-shrink: 0
ensures consistent button dimensions, andpointer-events: none
properly handles mouse interactions in the disabled state, enhancing the overall UX.Also applies to: 26-26
src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.styles.ts (2)
6-6
: Good addition of border-box sizing!This ensures consistent input dimensions by including padding and borders in the total width/height calculation.
32-41
: Verify potential text overlap with absolute positioning.The absolute positioning of
AddButtonContainer
might overlap with the 'Monerod Nodes URLs' label if it becomes too long (e.g., in other locales).❌ Verification inconclusive
🏁 Script executed:
#!/bin/bash # Search for translations of 'Monerod Nodes URLs' to check potential text length issues fd -e json -x jq -r 'select(."monerod-nodes-urls" != null) | ."monerod-nodes-urls"' {} \;Length of output: 153
Action Required: Manually Verify Translations for Label Lengths
The earlier automated check produced a JSON parse error and yielded no reliable output. This leaves us without conclusive evidence regarding potentially long translations for "Monerod Nodes URLs" that might cause the absolutely positioned AddButtonContainer to overlap.
To proceed, please run the following shell scripts to search for occurrences of the label key and its literal text across translation files:
#!/bin/bash # Search for occurrences of "monerod-nodes-urls" in all JSON files find . -type f -name "*.json" -exec grep -H "monerod-nodes-urls" {} \;#!/bin/bash # Also search for the literal label "Monerod Nodes URLs" in the repository grep -R "Monerod Nodes URLs" .Review the resulting outputs to check for any instances where the translation might be longer than the default text. If longer translations are detected, consider adjusting the container width or positioning in
src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.styles.ts
(lines 32–41) to prevent overlap.src/containers/floating/Settings/sections/experimental/MonerodMarkup/MonerodMarkup.tsx (1)
94-98
: Good layout improvement!Moving the add button to the top enhances visibility and usability.
src/containers/floating/Settings/sections/experimental/P2poolMarkup.tsx (1)
78-82
: Good error message centralization!Centralizing error message logic improves maintainability and consistency.
public/locales/en/settings.json (1)
53-54
: LGTM! The error messages are well organized.The new error message complements the existing
invalid-stats-server-port
message, providing more specific feedback when the port is empty.
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Cüzdanınız içe aktarıldığında Tari Universe otomatik olarak yeniden başlayacak", | |||
"invalid-seed-words": "Boşluklarla ayrılmış 24 kelime girin", | |||
"invalid-stats-server-port": "Geçersiz istatistik sunucu portu", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to Turkish.
The error message is currently in English while other messages in the file are in Turkish. This inconsistency affects the user experience for Turkish users.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "İstatistik sunucu portu boş olamaz",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "İstatistik sunucu portu boş olamaz", |
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Tari Universe sal outomaties herbegin sodra jou beursie ingevoer is", | |||
"invalid-seed-words": "Voer 24 woorde in, geskei deur spasies", | |||
"invalid-stats-server-port": "Ongeldige statistieke bedienerpoort", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to Afrikaans.
The error message is currently in English while other messages in the file are in Afrikaans. This inconsistency affects the user experience for Afrikaans users.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Statistieke bedienerpoort kan nie leeg wees nie",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Statistieke bedienerpoort kan nie leeg wees nie", |
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Tari Universe akan memulai ulang secara otomatis setelah dompet Anda diimpor", | |||
"invalid-seed-words": "Masukkan 24 kata yang dipisahkan oleh spasi", | |||
"invalid-stats-server-port": "Port server statistik tidak valid", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to Indonesian.
The error message is currently in English while other messages in the file are in Indonesian. This inconsistency affects the user experience for Indonesian users.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Port server statistik tidak boleh kosong",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Port server statistik tidak boleh kosong", |
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Tari Universe автоматически перезапустится после импорта вашего кошелька", | |||
"invalid-seed-words": "Введите 24 слова, разделенные пробелами", | |||
"invalid-stats-server-port": "Неверный порт сервера статистики", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to Russian.
The error message is currently in English while other messages in the file are in Russian. This inconsistency affects the user experience for Russian users.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Порт сервера статистики не может быть пустым",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Порт сервера статистики не может быть пустым", |
@@ -52,6 +52,7 @@ | |||
"importing-wallet-copy": "Tari Universe uruchomi się ponownie automatycznie po zaimportowaniu portfela", | |||
"invalid-seed-words": "Wprowadź 24 słowa oddzielone spacjami", | |||
"invalid-stats-server-port": "Nieprawidłowy port serwera statystyk", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to Polish.
The error message is currently in English while other messages in the file are in Polish. This inconsistency affects the user experience for Polish users.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Port serwera statystyk nie może być pusty",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Port serwera statystyk nie może być pusty", |
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Tari Universe wird automatisch neu starten, sobald dein Wallet importiert wurde", | |||
"invalid-seed-words": "Geben Sie 24 Wörter ein, getrennt durch Leerzeichen", | |||
"invalid-stats-server-port": "Ungültiger Statistik-Server-Port", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to German.
The error message is in English but should be translated to German to maintain consistency with other localized strings.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Statistik-Server-Port darf nicht leer sein",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Statistik-Server-Port darf nicht leer sein", |
@@ -51,6 +51,7 @@ | |||
"importing-wallet-copy": "Tari Universe redémarrera automatiquement une fois que votre portefeuille aura été importé", | |||
"invalid-seed-words": "Entrez 24 mots séparés par des espaces", | |||
"invalid-stats-server-port": "Port du serveur de statistiques invalide", | |||
"invalid-stats-server-port-empty": "Stats server port cannot be empty", |
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.
🛠️ Refactor suggestion
Translate the error message to French.
The error message is in English but should be translated to French to maintain consistency with other localized strings.
Apply this diff to translate the message:
- "invalid-stats-server-port-empty": "Stats server port cannot be empty",
+ "invalid-stats-server-port-empty": "Le port du serveur de statistiques ne peut pas être vide",
📝 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.
"invalid-stats-server-port-empty": "Stats server port cannot be empty", | |
"invalid-stats-server-port-empty": "Le port du serveur de statistiques ne peut pas être vide", |
@@ -76,6 +75,12 @@ const P2poolMarkup = () => { | |||
[currentStatsServerPort, editedCustomStatsServerPort, isRandomStatsServerPort] | |||
); | |||
|
|||
const errorMessage = !currentStatsServerInputPort |
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.
Nested ternary are hard to read, what about using just i18n context for example ?
const emptyContext = hasStatsServerPortError(currentStatsServerInputPort) ? "empty" : "notEmpty"
const errorMessage = !currentStatsServerInputPort ? t("invalid-stats-server-port", { context: emptyContext }) : null
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.
oooh nice, thank you @Misieq01 <3
updated!
also changed the contextSeparator
in i18init, to keep it consistent with our keys, but LMK if i should revert that and rather change the keys that will use context?
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.
One small comment otherwise looks great !
Description
currentStatsServerInputPort
value was 0 therefore invalid, even if you haven't actually added an invalid value - so added some checks and an extra error message for no value set+
add button to the top of the Monerod url section & included the remove buttons always (disabled if there's only one url) for alignmentMotivation and Context
How Has This Been Tested?
locally:
Summary by CodeRabbit
New Features
Bug Fixes
Style