fix: make i18n work for the newsletter page #3308
Conversation
Varun-Kolanu
requested review from
thulieblack,
quetzalliwrites,
asyncapi-bot-eve,
derberg,
akshatnema,
magicmatatjahu,
anshgoyalevil and
Mayaleeeee
as code owners
![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.
WalkthroughThe pull request introduces several changes across multiple files to enhance the newsletter subscription feature. The Changes
Assessment against linked issues
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (4)
🚧 Files skipped from review as they are similar to previous changes (4)
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
CodeRabbit Configuration File (
|
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify site configuration. |
asyncapi-bot
requested review from
devilkiller-ag,
sambhavgupta0705,
BhaswatiRoy,
J0SAL,
TRohit20 and
VaishnaviNandakumar
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-3308--asyncapi-website.netlify.app/ |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (5)
pages/[lang]/newsletter.tsx (1)
12-26: LGTM: Well-structured component with clear purpose.The
NewsletterIndexPagecomponent is implemented correctly, using appropriate layout components and setting the page title. It aligns well with the PR objectives.Consider enhancing the JSDoc comment to include information about the component's props (if any) and return value. For example:
/** * @description Component that is used on the landing page to embed newsletter and subscription option. * @returns {JSX.Element} The rendered NewsletterIndexPage component. */markdown/blog/automated-releases.md (1)
26-26: Consider minor style improvements.There are a couple of minor style suggestions from LanguageTool:
- Three successive sentences begin with "We". Consider rewording one of these sentences for better flow.
- The phrase "just recently" is redundant. Consider using either "just" or "recently".
These are minor suggestions and don't significantly impact the content, but addressing them could improve the overall readability of the paragraph.
🧰 Tools
🪛 LanguageTool
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
components/NewsletterSubscribe.tsx (3)
29-29: Document the newerrorSubtitlepropThe
errorSubtitleproperty has been added toNewsletterSubscribePropsbut is not documented in the component's JSDoc comments. Please add a description for this new property to maintain consistent documentation.Apply this diff to add the documentation:
/** * @description This component displays Newsletter Subscribe component. * * @param {NewsletterSubscribeProps} props - The props for the Newsletter Subscribe component. * @param {string} props.className - CSS class for styling the card. * @param {boolean} props.dark - If true, the theme of the component will be dark. * @param {string} props.title - The title of the Subscribe card. * @param {string} props.subtitle - The subtitle of the Subscribe card. * @param {string} props.type - The type of subscription. + * @param {string} props.errorSubtitle - The subtitle to display when an error occurs. */
48-48: Improve the defaulterrorSubtitlemessage for clarityThe default
errorSubtitlemessage could be rephrased for better clarity and grammar. Consider changing it to 'Subscription failed. Please let us know by submitting a bug report.'Apply this diff to update the default message:
- errorSubtitle = 'Subscription failed, please let us know about it by submitting a bug' + errorSubtitle = 'Subscription failed. Please let us know by submitting a bug report.'
117-117: Improve link text for better accessibilityUsing 'here' as link text is not descriptive for users relying on assistive technologies. Consider using more descriptive text such as 'submit a bug report'.
Apply this diff:
- {ready ? t('errorLinkText') : 'here'} + {ready ? t('errorLinkText') : 'submit a bug report'}Also, update the default value of
errorLinkTextin your translations or default props accordingly.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (20)
- components/NewsletterSubscribe.tsx (7 hunks)
- markdown/about/index.md (1 hunks)
- markdown/blog/2024-april-summary.md (1 hunks)
- markdown/blog/2024-august-summary.md (1 hunks)
- markdown/blog/2024-february-summary.md (1 hunks)
- markdown/blog/2024-july-summary.md (1 hunks)
- markdown/blog/2024-june-summary.md (1 hunks)
- markdown/blog/2024-march-summary.md (1 hunks)
- markdown/blog/2024-may-summary.md (1 hunks)
- markdown/blog/2024-september-summary-and-london.md (1 hunks)
- markdown/blog/april-2023.md (1 hunks)
- markdown/blog/august-2023.md (1 hunks)
- markdown/blog/automated-releases.md (1 hunks)
- markdown/blog/july-2023.md (1 hunks)
- markdown/blog/june-2023.md (1 hunks)
- markdown/blog/march-2023.md (1 hunks)
- markdown/blog/may-2023.md (1 hunks)
- markdown/blog/september-2023.md (1 hunks)
- pages/[lang]/newsletter.tsx (1 hunks)
- utils/i18n.ts (1 hunks)
✅ Files skipped from review due to trivial changes (12)
- markdown/about/index.md
- markdown/blog/2024-august-summary.md
- markdown/blog/2024-february-summary.md
- markdown/blog/2024-july-summary.md
- markdown/blog/2024-march-summary.md
- markdown/blog/2024-may-summary.md
- markdown/blog/april-2023.md
- markdown/blog/august-2023.md
- markdown/blog/july-2023.md
- markdown/blog/june-2023.md
- markdown/blog/may-2023.md
- markdown/blog/september-2023.md
🧰 Additional context used
🪛 LanguageTool
markdown/blog/automated-releases.md
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
🔇 Additional comments (6)
utils/i18n.ts (1)
10-11: LGTM! The changes enhance internationalization support for the newsletter.The addition of the '/newsletter' route to both 'en' and 'de' language paths in the
i18nPathsobject is consistent with the PR objectives. This change will support proper internationalization for the newsletter pages.Let's verify if these changes are consistent with the rest of the codebase:
Also applies to: 15-16
✅ Verification successful
Verification Successful! The
/newsletteradditions inutils/i18n.tsare consistent and correctly integrated across the codebase, supporting the intended internationalization enhancements.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any inconsistencies in newsletter route usage # Test 1: Check if there are any hardcoded references to '/newsletter' that should be updated echo "Checking for hardcoded newsletter routes:" rg '/newsletter' --type md --type tsx --type ts # Test 2: Verify if the NewsletterSubscribe component is using the correct import path for translations echo "Checking NewsletterSubscribe component import:" rg "from '.*utils/i18n'" --type tsx # Test 3: Look for any potential mismatches in language codes echo "Checking for language code consistency:" rg "\\b(en|de)\\b" --type ts --type tsxLength of output: 500
Script:
#!/bin/bash # Description: Re-run checks with explicit .tsx file extension # Test 1: Check if there are any hardcoded references to '/newsletter' that should be updated echo "Checking for hardcoded newsletter routes:" rg '/newsletter' -g '*.md' -g '*.tsx' -g '*.ts' # Test 2: Verify if the NewsletterSubscribe component is using the correct import path for translations echo "Checking NewsletterSubscribe component import:" rg "from '.*utils/i18n'" -g '*.tsx' # Test 3: Look for any potential mismatches in language codes echo "Checking for language code consistency:" rg "\\b(en|de)\\b" -g '*.ts' -g '*.tsx'Length of output: 6672
pages/[lang]/newsletter.tsx (2)
1-6: LGTM: Imports are well-organized and necessary.The imports are correctly structured, using aliased paths for better maintainability. All imported components and utilities are used in the file.
8-10: LGTM: Proper setup for static site generation with i18n.The static props and paths are correctly set up for Next.js with internationalization. The use of a utility function (
makeStaticProps) suggests a consistent approach across the project.markdown/blog/march-2023.md (1)
46-46: LGTM! Verify consistency across other files.The update to the newsletter subscription link is correct and aligns with the PR objectives for improved internationalization. The new link structure (
https://www.asyncapi.com/en/newsletter) properly includes the language code.Let's verify the consistency of this change across other relevant files:
This script will help ensure that the link has been updated consistently across all relevant files in the project.
✅ Verification successful
Newsletter link has been consistently updated across all relevant files.
No outdated newsletter links were found.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent newsletter link updates across relevant files # Test: Search for the new newsletter link structure echo "Files with updated newsletter link:" rg -l 'https://www.asyncapi.com/en/newsletter' # Test: Search for any remaining old newsletter link structure echo "\nFiles with potential outdated newsletter link:" rg -l 'https://www.asyncapi.com/newsletter'Length of output: 838
markdown/blog/2024-september-summary-and-london.md (1)
199-199: LGTM! Verify consistency across other files.The update of the newsletter URL to include the language prefix (/en/) is correct and aligns with the PR objectives for internationalization.
Let's verify if this change has been consistently applied across other relevant files:
✅ Verification successful
Consistent URL Updates Confirmed
All newsletter URLs have been successfully updated to include the
/en/prefix across the repository.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent newsletter URL updates across relevant files # Test: Search for both old and new URL patterns echo "Files still using the old URL pattern:" rg 'https://www\.asyncapi\.com/newsletter' --type md echo "\nFiles using the new URL pattern:" rg 'https://www\.asyncapi\.com/en/newsletter' --type mdLength of output: 4781
markdown/blog/automated-releases.md (1)
26-26: Newsletter link updated correctly.The newsletter link has been updated from
https://www.asyncapi.com/newslettertohttps://www.asyncapi.com/en/newsletter, which aligns with the PR objectives to update newsletter links across the project.🧰 Tools
🪛 LanguageTool
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
| import React from 'react'; | ||
|
|
||
| import Head from '@/components/Head'; | ||
| import Container from '@/components/layout/Container'; | ||
| import NewsletterSubscribe from '@/components/NewsletterSubscribe'; | ||
| import { getStaticPaths, makeStaticProps } from '@/utils/getStatic'; | ||
|
|
||
| const getStaticProps = makeStaticProps(['common']); | ||
|
|
||
| export { getStaticPaths, getStaticProps }; | ||
|
|
||
| /** | ||
| * @description component that is used on landing page to embed newsletter and subscription option. | ||
| */ | ||
| export default function NewsletterIndexPage() { | ||
| return ( | ||
| <div> | ||
| <Head title='Newsletter' /> | ||
| <div className='mt-12 py-12'> | ||
| <Container wide> | ||
| <NewsletterSubscribe /> | ||
| </Container> | ||
| </div> | ||
| </div> | ||
| ); | ||
| } |
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent import paths for NewsletterSubscribe component.
Several files import NewsletterSubscribe using both alias (@/components/NewsletterSubscribe) and relative paths (e.g., ../../components/NewsletterSubscribe). Please standardize the import paths to maintain consistency across the codebase.
🔗 Analysis chain
Great implementation of the newsletter page with i18n support.
This file successfully implements a new page for newsletter subscription, addressing the PR objectives. It follows React and Next.js best practices, including proper setup for internationalization. The code is clean, well-structured, and maintainable.
To ensure consistency across the project, let's verify the usage of the NewsletterSubscribe component:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent usage of NewsletterSubscribe component
# Test: Search for other usages of NewsletterSubscribe
echo "Checking for other usages of NewsletterSubscribe:"
rg "NewsletterSubscribe" --type tsx --type ts
# Test: Verify the import path for NewsletterSubscribe
echo "\nVerifying import path for NewsletterSubscribe:"
rg "import.*NewsletterSubscribe.*from" --type tsx --type ts
Length of output: 379
Script:
#!/bin/bash
# Description: Check for consistent usage of NewsletterSubscribe component
# Test: Search for other usages of NewsletterSubscribe in .ts and .tsx files
echo "Checking for other usages of NewsletterSubscribe:"
rg "NewsletterSubscribe" --glob "*.ts" --glob "*.tsx"
# Test: Verify the import path for NewsletterSubscribe in .ts and .tsx files
echo "\nVerifying import path for NewsletterSubscribe:"
rg "import.*NewsletterSubscribe.*from" --glob "*.ts" --glob "*.tsx"
Length of output: 3152
markdown/blog/2024-april-summary.md
Outdated
| @@ -45,4 +45,4 @@ We're thrilled to announce a new automated TSC voting system! We want to thank [ | |||
|
|
|||
| ## Coming in May | |||
| - **AACoT'24 Helsinki** - The Helsinki Edition will take place on May 28th and 29th. Please keep an eye out for announcements on community tickets on Slack and our social channels. | |||
| - **AsyncAPI Community Updates Newsletter** - The May Edition issue will arrive in your inbox this coming week. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/newsletter); you don't want to miss out. | |||
| - **AsyncAPI Community Updates Newsletter** - The May Edition issue will arrive in your inbox this coming week. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/en/newsletter); you don't want to miss out. | |||
There was a problem hiding this comment.
💡 Codebase verification
Inconsistent Newsletter Links Found
The newsletter link was updated in markdown/blog/2024-april-summary.md, but several other markdown files still use the old /newsletter link. Please update the newsletter links consistently across all files to ensure all users have access to the correct subscription page.
Files still using the old newsletter link:
- markdown/about/index.md
- markdown/blog/march-2023.md
- markdown/blog/september-2023.md
- markdown/blog/august-2023.md
- markdown/blog/april-2023.md
- markdown/blog/2024-september-summary-and-london.md
- markdown/blog/2024-may-summary.md
- markdown/blog/2024-march-summary.md
- markdown/blog/automated-releases.md
- markdown/blog/2024-june-summary.md
- markdown/blog/june-2023.md
- markdown/blog/2024-april-summary.md
- markdown/blog/2024-july-summary.md
- markdown/blog/may-2023.md
- markdown/blog/2024-february-summary.md
- markdown/blog/2024-august-summary.md
- markdown/blog/july-2023.md
- markdown/blog/march-2023.md
🔗 Analysis chain
LGTM! Consistent update of the newsletter link.
The change from /newsletter to /en/newsletter aligns with the PR objectives and internationalization efforts. This update ensures that the English version of the newsletter page is correctly linked.
Let's verify the consistency of this change across other relevant files:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent updates of the newsletter link across markdown files
# Test: Search for both old and new newsletter links in markdown files
echo "Files still using the old newsletter link:"
rg '/newsletter\)' -g '*.md'
echo "Files using the new newsletter link:"
rg '/en/newsletter\)' -g '*.md'
Length of output: 9248
markdown/blog/2024-june-summary.md
Outdated
| @@ -76,4 +76,4 @@ If you are interested in joining the program or want to know more about our curr | |||
|
|
|||
| - **AACoT'24 London CFP Deadline** - [The Call for Speakers for AACoT'24 London Edition](https://conference.asyncapi.com/venue/london) will close on the 12th of July. Please submit your proposal soon and prepare to share your knowledge with the community. | |||
|
|
|||
| - **AsyncAPI Community Updates Newsletter** - The July Edition issue will arrive in your inbox on Thursday the 4th. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/newsletter). | |||
| - **AsyncAPI Community Updates Newsletter** - The July Edition issue will arrive in your inbox on Thursday the 4th. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/en/newsletter). | |||
There was a problem hiding this comment.
💡 Codebase verification
Newsletter link is broken (404 Not Found)
The updated newsletter subscription link returns a 404 error. Please verify the correct URL or update it to a valid newsletter page.
markdown/blog/2024-june-summary.mdline 79
🔗 Analysis chain
LGTM: Newsletter link updated correctly.
The change to the newsletter subscription link aligns with the PR objectives and is consistent with similar updates mentioned in the AI summary. This modification supports proper internationalization by directing users to the English version of the newsletter page.
A few points to consider:
- The link appears to be correct, assuming the English version of the newsletter page exists at
/en/newsletter. - This change improves consistency across the website for newsletter subscriptions.
- For full internationalization support, consider adding a note or logic to direct users to localized versions of the newsletter page if available.
To ensure the link is functional and the page exists, please run the following script:
This script will check if the page returns a successful HTTP status code.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the existence of the English newsletter page
curl -I https://www.asyncapi.com/en/newsletter
Length of output: 768
Varun-Kolanu
force-pushed
the
newsletter
branch
from
07ff3e0
to
05cac5a
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
markdown/blog/automated-releases.md (1)
26-26: Consider minor style improvementsThere are a couple of minor style suggestions to consider:
- Three successive sentences begin with "We". Consider rewording one of them for better flow.
- The phrase "just recently" is redundant. Consider using either "just" or "recently".
These are minor suggestions and don't impact the overall content quality.
🧰 Tools
🪛 LanguageTool
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (18)
- components/NewsletterSubscribe.tsx (7 hunks)
- markdown/about/index.md (1 hunks)
- markdown/blog/2024-april-summary.md (1 hunks)
- markdown/blog/2024-august-summary.md (1 hunks)
- markdown/blog/2024-february-summary.md (1 hunks)
- markdown/blog/2024-july-summary.md (1 hunks)
- markdown/blog/2024-june-summary.md (1 hunks)
- markdown/blog/2024-march-summary.md (1 hunks)
- markdown/blog/2024-may-summary.md (1 hunks)
- markdown/blog/2024-september-summary-and-london.md (1 hunks)
- markdown/blog/april-2023.md (1 hunks)
- markdown/blog/august-2023.md (1 hunks)
- markdown/blog/automated-releases.md (1 hunks)
- markdown/blog/july-2023.md (1 hunks)
- markdown/blog/june-2023.md (1 hunks)
- markdown/blog/march-2023.md (1 hunks)
- markdown/blog/may-2023.md (1 hunks)
- markdown/blog/september-2023.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (16)
- components/NewsletterSubscribe.tsx
- markdown/about/index.md
- markdown/blog/2024-april-summary.md
- markdown/blog/2024-august-summary.md
- markdown/blog/2024-february-summary.md
- markdown/blog/2024-july-summary.md
- markdown/blog/2024-june-summary.md
- markdown/blog/2024-march-summary.md
- markdown/blog/2024-may-summary.md
- markdown/blog/2024-september-summary-and-london.md
- markdown/blog/april-2023.md
- markdown/blog/august-2023.md
- markdown/blog/july-2023.md
- markdown/blog/june-2023.md
- markdown/blog/may-2023.md
- markdown/blog/september-2023.md
🧰 Additional context used
🪛 LanguageTool
markdown/blog/automated-releases.md
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
markdown/blog/march-2023.md
[uncategorized] ~46-~46: A comma might be missing here.
Context: ...loop of the activities happening in the community such as discussions, meetings, and upda...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
🔇 Additional comments (3)
markdown/blog/march-2023.md (2)
46-46: LGTM: Newsletter subscription link updated correctly.The newsletter subscription link has been updated to include the language prefix ('/en/'), which aligns with the PR objectives of implementing proper i18n guidelines. This change is consistent with similar updates in other files and supports the internationalization efforts.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: A comma might be missing here.
Context: ...loop of the activities happening in the community such as discussions, meetings, and upda...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
46-46: No action needed: Comma suggestion is a false positive.The static analysis tool suggested a possible missing comma after "community". However, this suggestion is a false positive. The current sentence structure is grammatically correct and adding a comma would slightly alter the meaning. The existing format effectively conveys the intended message without the need for an additional comma.
🧰 Tools
🪛 LanguageTool
[uncategorized] ~46-~46: A comma might be missing here.
Context: ...loop of the activities happening in the community such as discussions, meetings, and upda...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
markdown/blog/automated-releases.md (1)
26-26: LGTM: Newsletter link updated for i18n supportThe newsletter link has been updated from
https://www.asyncapi.com/newslettertohttps://www.asyncapi.com/en/newsletter. This change aligns with the PR objectives to enhance internationalization (i18n) functionality.🧰 Tools
🪛 LanguageTool
[style] ~26-~26: Three successive sentences begin with the same word. Consider rewording the sentence or use a thesaurus to find a synonym.
Context: ...arted to improve our tooling regularly. We are now periodically sharing project st...(ENGLISH_WORD_REPEAT_BEGINNING_RULE)
[style] ~26-~26: This phrase is redundant. Consider writing “just” or “recently”.
Context: ...ues/115), but most important is that we just recently updated our roadmap. Am I just showing...(JUST_RECENTLY)
markdown/blog/2024-april-summary.md
Outdated
| @@ -45,4 +45,4 @@ We're thrilled to announce a new automated TSC voting system! We want to thank [ | |||
|
|
|||
| ## Coming in May | |||
| - **AACoT'24 Helsinki** - The Helsinki Edition will take place on May 28th and 29th. Please keep an eye out for announcements on community tickets on Slack and our social channels. | |||
| - **AsyncAPI Community Updates Newsletter** - The May Edition issue will arrive in your inbox this coming week. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/newsletter); you don't want to miss out. | |||
| - **AsyncAPI Community Updates Newsletter** - The May Edition issue will arrive in your inbox this coming week. Ensure [you subscribe to the AsyncAPI Newsletter](https://www.asyncapi.com/en/newsletter); you don't want to miss out. | |||
There was a problem hiding this comment.
Do we actually require /en/ route here or can we just add simple /newsletter route, and the language will be decided by website server?
There was a problem hiding this comment.
Yeah that is a better one, thanks, updating!
There was a problem hiding this comment.
Have updated, now it automatically redirects to appropriate route by detecting. Actually I was confused by seeing the tools/cli page's implementation because detection wasn't present in that. Later main page cleared the way
Varun-Kolanu
force-pushed
the
newsletter
branch
from
05cac5a
to
3deec31
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (4)
- components/NewsletterSubscribe.tsx (7 hunks)
- pages/[lang]/newsletter.tsx (1 hunks)
- pages/newsletter.tsx (1 hunks)
- utils/i18n.ts (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- components/NewsletterSubscribe.tsx
- pages/[lang]/newsletter.tsx
- utils/i18n.ts
🧰 Additional context used
🔇 Additional comments (2)
pages/newsletter.tsx (2)
Line range hint
1-24: Verify i18n implementation in this component.The PR objectives mention improvements to i18n functionality, but there are no visible i18n-related changes in this file. Ensure that the necessary i18n features are implemented correctly.
#!/bin/bash # Description: Check for i18n-related imports and usage in the NewsletterIndexPage component # Expected result: The component should use i18n features for text content # Test 1: Check for i18n-related imports rg "import.*i18n" pages/newsletter.tsx # Test 2: Check for usage of translation functions rg "useTranslation|t\(" pages/newsletter.tsx # Test 3: Check if the NewsletterSubscribe component handles i18n rg -A 10 "export default function NewsletterSubscribe" components/NewsletterSubscribe.tsx
6-6: Verify the purpose and impact of the new Redirect import.The addition of the
Redirectimport suggests a change in the page's behavior. Ensure that this aligns with the intended i18n improvements mentioned in the PR objectives.
pages/newsletter.tsx
Outdated
| Redirect(); | ||
|
|
There was a problem hiding this comment.
Potential issue: Unconditional redirect may lead to unreachable code.
The Redirect() function is called unconditionally at the beginning of the component. This could prevent the rest of the component from rendering, making the subsequent JSX code unreachable.
Consider wrapping the Redirect() call in a condition or effect to ensure it's used appropriately. For example:
import { useEffect } from 'react';
export default function NewsletterIndexPage() {
useEffect(() => {
Redirect();
}, []);
// Rest of the component...
}This change would allow the component to render initially and then perform the redirect, which might be necessary for client-side navigation or for allowing any cleanup operations to run.
Varun-Kolanu
force-pushed
the
newsletter
branch
from
3deec31
to
af8e3a6
Compare
Varun-Kolanu
force-pushed
the
newsletter
branch
2 times, most recently
from
b995b17
to
2a79c48
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #3308 +/- ##
=======================================
Coverage 67.77% 67.77%
=======================================
Files 21 21
Lines 664 664
=======================================
Hits 450 450
Misses 214 214 ☔ View full report in Codecov by Sentry. |
Varun-Kolanu
force-pushed
the
newsletter
branch
from
2a79c48
to
82edcd9
Compare
|
@Varun-Kolanu @akshatnema is there anything pending on this PR? It would be nice to have the labels showing correctly 🙏 Thank you so much! |
The work is done imo and this PR fixes the issue. I'm just waiting for a maintainer to review and merge. |
|
Hey @Varun-Kolanu @akshatnema , on the
Rest everything seems fine to me. |
|
Hey @SahilDahekar, actually there are other pages too which need to have i18n enabled. As the issue revolved around making newsletter page, I just made the changes for that page. I think this might be opened as a new issue or if @akshatnema suggests to make all the pages work for i18n, I may add changes for those pages too |
Varun-Kolanu
force-pushed
the
newsletter
branch
from
3042572
to
5a3a835
Compare
Description
entext/en/newsletter and /newsletter:
/de/newsletter:
Implementation details
pages/[lang]folder, edited it by adding scripts into it, to properly use the language selected by user and hence can see the corresponding text.Redirects()used in thepages/newsletter.tsx.components/NewsLetterSubscribe.tsxto adapt to both types of pages by using thereadyvariable returned from useTranslation.Related issue
Fixes #3236
The issue of labels in buttons and placeholder existed because, the i18n implementation was not working then. As this PR properly implements i18n, that issue of buttons too got eliminated!
Summary by CodeRabbit
Summary by CodeRabbit
New Features
Improvements
Bug Fixes