GoogleCalender Button, ics File Download Button: text fixed in community/events #3471
Conversation
Shubhashish-Chakraborty
requested review from
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil,
Mayaleeeee and
asyncapi-bot-eve
as code owners
WalkthroughThe pull request modifies two button components: Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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
|
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Welcome to AsyncAPI. Thanks a lot for creating your first pull request. Please check out our contributors guide useful for opening a pull request.
Keep in mind there are also other channels you can use to interact with AsyncAPI community. For more details check out this issue.
|
We require all PRs to follow Conventional Commits specification. |
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Outside diff range and nitpick comments (2)
components/buttons/ICSFileButton.tsx (2)
10-11: Remove extra blank lineThere's an unnecessary extra blank line here.
import Button from './Button'; - // Importing text to display on button from common.json🧰 Tools
🪛 eslint
[error] 1-11: Run autofix to sort these imports!
(simple-import-sort/imports)
Line range hint
11-24: Refactor the localization implementationThe current implementation directly imports text from the English locale file, which bypasses the i18n system and makes the component less maintainable. Since you're already using the
useTranslationhook, you should use translation keys instead of importing the text directly.Apply this diff to fix the localization:
-// Importing text to display on button from common.json -import { icsFileBtn } from '../../public/locales/en/common.json'; interface IICSFButtonProps extends IButtonDefaultProps {} /** * @description The ICSFButton component is a button that links to an ICS file. * @param {string} props.text - The text to display on the button. * @param {string} props.href - The href attribute for the anchor tag. * @param {string} props.target - The target attribute for the anchor tag. * @param {ButtonIconPosition} props.iconPosition - The position of the icon in the button. * @param {string} props.className - The class name to be applied to the button. */ export default function ICSFButton({ - text = icsFileBtn, + text = 'icsFileBtn',This change:
- Removes direct dependency on the English locale file
- Uses a translation key that can be defined in all locale files
- Maintains consistency with i18n best practices
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/buttons/GoogleCalendarButton.tsx(2 hunks)components/buttons/ICSFileButton.tsx(2 hunks)
🧰 Additional context used
🪛 eslint
components/buttons/GoogleCalendarButton.tsx
[error] 10-11: Delete ⏎
(prettier/prettier)
[error] 10-11: More than 1 blank line not allowed.
(no-multiple-empty-lines)
🔇 Additional comments (1)
components/buttons/GoogleCalendarButton.tsx (1)
Line range hint 1-40: Verify localization keys exist in all locale files
Since both components are being updated to use specific translation keys, we should verify these keys exist in all locale files to prevent missing translations.
✅ Verification successful
The script results show that both googleCalendarBtn and icsFileBtn translation keys exist in both English (en) and German (de) locale files, as no warnings were generated. However, I notice a discrepancy between the code and our verification. The code is using githubButton as the default text value, but we checked for googleCalendarBtn. Let me verify this specific key.
All required translation keys are present and properly configured
The verification shows that both locale files (English and German) contain all the necessary translation keys:
githubButton(currently used as default text)googleCalendarBtn(available for use)icsFileBtn
All keys have appropriate translations in both languages, ensuring proper localization support.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the required translation keys exist in all locale files
# Find all locale files
echo "Checking locale files for required translation keys..."
for locale_file in $(find public/locales -name "common.json"); do
echo "Checking $locale_file"
# Check for both keys
jq 'select(.googleCalendarBtn and .icsFileBtn)' "$locale_file" > /dev/null
if [ $? -ne 0 ]; then
echo "Warning: Missing required translation keys in $locale_file"
echo "Required keys: googleCalendarBtn, icsFileBtn"
echo "Found keys:"
jq 'keys' "$locale_file"
fi
done
Length of output: 657
Script:
#!/bin/bash
# Check for the actual key being used in the code
echo "Checking for githubButton key in locale files..."
for locale_file in $(find public/locales -name "common.json"); do
echo "Checking $locale_file"
jq '.githubButton' "$locale_file"
done
# Also display the full content of common.json files to see available keys
echo -e "\nFull content of locale files:"
for locale_file in $(find public/locales -name "common.json"); do
echo -e "\n=== $locale_file ==="
cat "$locale_file"
done
Length of output: 3923
🧰 Tools
🪛 eslint
[error] 10-11: Delete ⏎
(prettier/prettier)
[error] 10-11: More than 1 blank line not allowed.
(no-multiple-empty-lines)
|
|
||
| // Importing text to display on button from common.json | ||
| import { githubButton } from '../../public/locales/en/common.json'; | ||
|
|
There was a problem hiding this comment.
Remove extra blank line and fix incorrect import
There are two issues here:
- Extra blank line
- Incorrect text key import (
githubButtonfor a Google Calendar button)
import Button from './Button';
-
-// Importing text to display on button from common.json
-import { githubButton } from '../../public/locales/en/common.json';📝 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.
| // Importing text to display on button from common.json | |
| import { githubButton } from '../../public/locales/en/common.json'; | |
| import Button from './Button'; |
🧰 Tools
🪛 eslint
[error] 10-11: Delete ⏎
(prettier/prettier)
[error] 10-11: More than 1 blank line not allowed.
(no-multiple-empty-lines)
| @@ -18,7 +22,7 @@ | |||
| * @param {string} props.className - The class name to be applied to the button. | |||
| */ | |||
| export default function GoogleCalendarButton({ | |||
| text = 'googleCalendarBtn', | |||
| text = githubButton, | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix incorrect text key and refactor localization
The component is using githubButton for a Google Calendar button, which is incorrect. Additionally, like the ICS button, this implementation bypasses the i18n system.
- text = githubButton,
+ text = 'googleCalendarBtn',This change:
- Uses the correct translation key for a Google Calendar button
- Removes direct dependency on the English locale file
- Maintains consistency with i18n best practices
Committable suggestion skipped: line range outside the PR's diff.
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3471--asyncapi-website.netlify.app/ |
|
closing this as there is already a PR present for this |
I've tried to fix the issue and raised the PR here: #3465 , Please review it.
Changes made in the
website/components/buttons/inGoogleCalenderButtonandICSFileButtonComponent:This PR attempts to address the issue with a simpler approach that I’ve implemented. While this might not be perfect, I kindly request a thorough review of the code. Please let me know if this approach is correct or if there are areas for improvement. I’m eager to learn and refine based on your feedback.
@anshgoyalevil @derberg
Summary by CodeRabbit
GoogleCalendarButtonto use localized text from an external JSON file.ICSFileButtonto use localized text from an external JSON file.