Skip to content
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: make i18n work for the newsletter page #3308

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Varun-Kolanu
Copy link

@Varun-Kolanu Varun-Kolanu commented Oct 21, 2024

Description

  1. Updated proper internationalization for the newsletter page by following the guidelines given in documentation.
  2. /en/newsletter and /de/newsletter works well, /newsletter page defaults to defined en text

/en/newsletter and /newsletter:
image

/de/newsletter:
image

Implementation details

  • The previous newsletter page used i18n but didn't have the parent element to initialize them.
  • Then I have copied the newsletter page into 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.
  • Now the user can go to i18n page. If /newsletter is hit, server detects the language and takes them to the appropriate page automatically, due to the Redirects() used in the pages/newsletter.tsx.
  • I have edited the components/NewsLetterSubscribe.tsx to adapt to both types of pages by using the ready variable returned from useTranslation.
  • As only newsletterCTA is being used in that component from the locale file, I have used { keyprefix: 'newsletterCTA' } in the useTranslation so that we can access the values directly.

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

    • Introduced a new landing page for newsletter subscriptions, enhancing user accessibility.
    • Added an optional error message feature in the newsletter subscription component for improved user feedback.
  • Improvements

    • Enhanced internationalization support for the newsletter subscription component, ensuring translations are displayed correctly.
    • Added new routing capabilities for the newsletter in multiple languages.
  • Bug Fixes

    • Implemented redirection logic in the newsletter index page to streamline user navigation.

Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link

coderabbitai bot commented Oct 21, 2024

Walkthrough

The pull request introduces several changes across multiple files to enhance the newsletter subscription feature. The NewsletterSubscribe component now supports a custom error message through a new optional property, errorSubtitle, and improves translation readiness. A new landing page component, NewsletterIndexPage, is added to facilitate the subscription interface, alongside updates to routing for internationalization. Additionally, a redirection mechanism is implemented in the existing newsletter page to modify user navigation.

Changes

File Change Summary
components/NewsletterSubscribe.tsx Updated import for useTranslation, added errorSubtitle to props, enhanced translation logic.
pages/[lang]/newsletter.tsx Introduced NewsletterIndexPage component, added getStaticProps and getStaticPaths.
utils/i18n.ts Added /newsletter route to i18nPaths for English and German.
pages/newsletter.tsx Added Redirect function to control flow in NewsletterIndexPage.

Assessment against linked issues

Objective Addressed Explanation
Correct button labels in the newsletter page (#3236) The changes do not address the button label issue.

Suggested labels

ready-to-merge

Suggested reviewers

  • derberg
  • magicmatatjahu
  • sambhavgupta0705
  • akshatnema
  • anshgoyalevil

Poem

In the meadow where bunnies play,
A newsletter blooms bright as day.
With labels now clear, and messages right,
Our subscriptions take off, a joyful flight! 🐇✨
Hops of delight, with translations anew,
Come join us, dear friends, there's room for you!


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3deec31 and af8e3a6.

📒 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 (4)
  • components/NewsletterSubscribe.tsx
  • pages/[lang]/newsletter.tsx
  • pages/newsletter.tsx
  • utils/i18n.ts

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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

netlify bot commented Oct 21, 2024

Deploy Preview for asyncapi-website ready!

Built without sensitive environment variables

Name Link
🔨 Latest commit af8e3a6
🔍 Latest deploy log https://app.netlify.com/sites/asyncapi-website/deploys/67190abd64cba90008b156c8
😎 Deploy Preview https://deploy-preview-3308--asyncapi-website.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@Varun-Kolanu Varun-Kolanu changed the title Newsletter fix: make i18n work for the newsletter page Oct 21, 2024
@asyncapi-bot
Copy link
Contributor

asyncapi-bot commented Oct 21, 2024

⚡️ Lighthouse report for the changes in this PR:

Category Score
🔴 Performance 41
🟢 Accessibility 98
🟢 Best practices 92
🟢 SEO 100
🔴 PWA 33

Lighthouse ran on https://deploy-preview-3308--asyncapi-website.netlify.app/

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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 NewsletterIndexPage component 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:

  1. Three successive sentences begin with "We". Consider rewording one of these sentences for better flow.
  2. 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 new errorSubtitle prop

The errorSubtitle property has been added to NewsletterSubscribeProps but 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 default errorSubtitle message for clarity

The default errorSubtitle message 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 accessibility

Using '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 errorLinkText in your translations or default props accordingly.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 3a8f69a and 07ff3e0.

📒 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 i18nPaths object 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 /newsletter additions in utils/i18n.ts are 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 tsx

Length 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 md

Length 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/newsletter to https://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)

Comment on lines +1 to +26
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>
);
}
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

@@ -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.
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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

@@ -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).
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 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.md line 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:

  1. The link appears to be correct, assuming the English version of the newsletter page exists at /en/newsletter.
  2. This change improves consistency across the website for newsletter subscriptions.
  3. 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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

🧹 Outside diff range and nitpick comments (1)
markdown/blog/automated-releases.md (1)

26-26: Consider minor style improvements

There are a couple of minor style suggestions to consider:

  1. Three successive sentences begin with "We". Consider rewording one of them for better flow.
  2. 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

📥 Commits

Files that changed from the base of the PR and between 07ff3e0 and 05cac5a.

📒 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 support

The newsletter link has been updated from https://www.asyncapi.com/newsletter to https://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)

@@ -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.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we actually require /en/ route here or can we just add simple /newsletter route, and the language will be decided by website server?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah that is a better one, thanks, updating!

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Files that changed from the base of the PR and between 05cac5a and 3deec31.

📒 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 Redirect import suggests a change in the page's behavior. Ensure that this aligns with the intended i18n improvements mentioned in the PR objectives.

Comment on lines +12 to +13
Redirect();

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[BUG] Newsletter page does not have correct button labels
3 participants