-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
fix: update Slack invite links to open external URL in new tab #4841
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
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for asyncapi-website ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
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.
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.
📝 WalkthroughWalkthroughUpdated the Slack invite link in the community page component from https://asyncapi.com/slack-invite to https://www.asyncapi.com/slack-invite. Added minor whitespace formatting to the redirect configuration file. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes Poem
Pre-merge checks❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
📜 Recent review detailsConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4841--asyncapi-website.netlify.app/ |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/buttons/SlackButton.tsx (1)
1-1: Address pipeline failures before merge.The CI pipeline has flagged two formatting issues:
- Line 1: Import sorting violation - run autofix to sort imports
- Line 33: Remove trailing newline
🔎 Run the following command to auto-fix formatting issues
npm run formatOr manually apply these fixes:
- Sort imports at the top of the file
- Remove the trailing newline at the end
Also applies to: 33-33
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
components/buttons/SlackButton.tsxpages/[lang]/index.tsxpages/community/index.tsxpublic/_redirects
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
pages/community/index.tsxcomponents/buttons/SlackButton.tsxpages/[lang]/index.tsx
🧬 Code graph analysis (1)
pages/[lang]/index.tsx (1)
components/buttons/Button.tsx (1)
Button(53-106)
🪛 GitHub Actions: PR testing - if Node project
components/buttons/SlackButton.tsx
[error] 1-1: simple-import-sort/imports: Run autofix to sort these imports
[error] 33-33: prettier/prettier: Delete trailing newline
pages/[lang]/index.tsx
[error] 104-104: prettier/prettier: Delete the space character
[error] 105-105: prettier/prettier: Delete unnecessary indentation
[error] 106-106: prettier/prettier: Delete unnecessary indentation
[error] 107-107: prettier/prettier: Replace href with single quotes and correct spacing
[error] 107-107: jsx-quotes: Unexpected usage of doublequote
[error] 108-108: prettier/prettier: Replace target attribute with single quotes
[error] 108-108: jsx-quotes: Unexpected usage of doublequote
[error] 109-109: prettier/prettier: Replace rel attribute with single quotes
[error] 109-109: jsx-quotes: Unexpected usage of doublequote
[error] 110-110: prettier/prettier: Delete unnecessary space
[error] 111-111: prettier/prettier: Delete trailing newline
🔇 Additional comments (4)
components/buttons/SlackButton.tsx (2)
15-15: LGTM! External Slack invite URL is now the default.The updated default href correctly points to the external Slack invite URL, ensuring consistent behavior across all uses of the SlackButton component.
29-29: Good catch on the spacing fix!The double space between
bg-slackandhover:bg-slack-lighthas been corrected.pages/community/index.tsx (1)
179-179: LGTM! Slack invite link updated to external URL.The HomeCards component now correctly links to the external Slack invite URL, consistent with the changes across the codebase.
public/_redirects (1)
56-57: LGTM! Redirect consolidated to use the same external URL.The
/slack-inviteredirect now points towww.asyncapi.com/slack-invite, which matches the URL used throughout the UI components. This provides a single point of control for managing the Slack invite link.Using a 302 (temporary) redirect is appropriate for an external service URL that may change.
pages/[lang]/index.tsx
Outdated
| <Button | ||
| className='w-full md:w-auto' | ||
| text={t('community.slackCTABtn')} | ||
| href="https://www.asyncapi.com/slack-invite" | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| /> | ||
| </div> | ||
|
|
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.
Fix formatting issues and remove redundant attribute.
The logic is correct - the Slack CTA now opens the external invite URL in a new tab. However, there are multiple formatting violations:
Issues:
- Double quotes: JSX attributes should use single quotes per project conventions
- Indentation: Lines 105-110 have unnecessary indentation
- Spacing: Extra spaces around attributes
- Trailing newline: Line 112 has an unnecessary blank line
- Redundant attribute: The
rel="noopener noreferrer"is already added by the Button component whentargetis set (see Button.tsx line 93)
🔎 Proposed fix with correct formatting
- <Button
- className='w-full md:w-auto'
- text={t('community.slackCTABtn')}
- href="https://www.asyncapi.com/slack-invite"
- target="_blank"
- rel="noopener noreferrer"
- />
-
+ <Button
+ className='w-full md:w-auto'
+ text={t('community.slackCTABtn')}
+ href='https://www.asyncapi.com/slack-invite'
+ target='_blank'
+ />Note: Removed rel="noopener noreferrer" since the Button component already applies it automatically for links with target set.
You can also run npm run format to auto-fix these issues.
📝 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.
| <Button | |
| className='w-full md:w-auto' | |
| text={t('community.slackCTABtn')} | |
| href="https://www.asyncapi.com/slack-invite" | |
| target="_blank" | |
| rel="noopener noreferrer" | |
| /> | |
| </div> | |
| <Button | |
| className='w-full md:w-auto' | |
| text={t('community.slackCTABtn')} | |
| href='https://www.asyncapi.com/slack-invite' | |
| target='_blank' | |
| /> | |
| </div> |
🧰 Tools
🪛 GitHub Actions: PR testing - if Node project
[error] 104-104: prettier/prettier: Delete the space character
[error] 105-105: prettier/prettier: Delete unnecessary indentation
[error] 106-106: prettier/prettier: Delete unnecessary indentation
[error] 107-107: prettier/prettier: Replace href with single quotes and correct spacing
[error] 107-107: jsx-quotes: Unexpected usage of doublequote
[error] 108-108: prettier/prettier: Replace target attribute with single quotes
[error] 108-108: jsx-quotes: Unexpected usage of doublequote
[error] 109-109: prettier/prettier: Replace rel attribute with single quotes
[error] 109-109: jsx-quotes: Unexpected usage of doublequote
[error] 110-110: prettier/prettier: Delete unnecessary space
[error] 111-111: prettier/prettier: Delete trailing newline
🤖 Prompt for AI Agents
In pages/[lang]/index.tsx around lines 104 to 112, fix JSX formatting and remove
the redundant rel attribute: change double quotes to single quotes for JSX
attributes, normalize indentation of the Button props to match project style,
remove extra spaces around attributes, delete the redundant rel="noopener
noreferrer" (Button.tsx already adds it when target is present), and remove the
trailing blank line at line 112; run the formatter (npm run format) after making
these edits.
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #4841 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 22 22
Lines 798 798
Branches 146 146
=========================================
Hits 798 798 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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: 1
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/buttons/SlackButton.tsxpages/[lang]/index.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- pages/[lang]/index.tsx
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-12-29T14:21:28.216Z
Learnt from: sammy200-ui
Repo: asyncapi/website PR: 4804
File: components/navigation/Filter.tsx:32-41
Timestamp: 2025-12-29T14:21:28.216Z
Learning: In the asyncapi/website repository, when you intentionally omit dependencies in React hooks (e.g., useEffect, useMemo), add an eslint-disable comment with an explanatory note above the line to justify the design choice. For example: // eslint-disable-next-line react-hooks/exhaustive-deps: intentionally omitting dependencies to avoid unnecessary re-runs. This helps reviewers understand the rationale and keeps lint guidance informative.
Applied to files:
components/buttons/SlackButton.tsx
🧬 Code graph analysis (1)
components/buttons/SlackButton.tsx (2)
components/buttons/Button.tsx (1)
Button(53-106)components/icons/Slack.tsx (1)
IconSlack(7-20)
⏰ Context from checks skipped due to timeout of 180000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Test NodeJS PR - windows-latest
🔇 Additional comments (2)
components/buttons/SlackButton.tsx (2)
11-13: LGTM: Clear documentation.The JSDoc accurately describes the component's behavior, making it clear that the button always opens the Slack invite in a new tab.
14-15: LGTM: Good API simplification.The simplified API (accepting only
textandclassName) makes the component more focused and ensures consistent Slack invite behavior. Using a const for the href is clean and maintainable.Note: This is a breaking change if the component was previously used with custom
href,target, oriconPositionprops, but this appears intentional based on the PR objectives.
| build with AsyncAPI." | ||
| btnText='Join AsyncAPI slack' | ||
| link='https://asyncapi.com/slack-invite' | ||
| link='https://www.asyncapi.com/slack-invite' |
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.
Why we need to include a www cname?
| /asyncapi-react https://asyncapi.github.io/asyncapi-react 301! | ||
|
|
||
| # Slack | ||
| /slack-invite https://join.slack.com/t/asyncapi/shared_invite/zt-3m4pmrguv-SUN9Js4BkQHocIH54F59sA 302! |
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.
I feel like this would result in a redirect loop. why are we removing the slack invite link here?
|
Thanks for the review ! After digging deeper, I realized the /slack-invite route is intentionally handled via Netlify redirects and is expected to 404 in local dev while working correctly in production. Since there’s no user facing issue, I’ve restored the original behavior and reverted my changes. Appreciate the guidance this helped me better understand the redirect setup and existing abstractions. |
🛠️ Fix Slack Invite Redirects
This PR ensures that all Slack invite buttons correctly open the external Slack invite page instead of redirecting to a 404 in local development.
✔ What’s Updated
Replaced
/slack-invitepaths with the correct external invite URL:https://www.asyncapi.com/slack-inviteUpdated:
Homepage Slack CTA button
Community page Slack CTA button
SlackButton component behavior
All Slack links now open in a new browser tab (
target="_blank")🧪 Verification
🔍 Notes
No related issue. Small, isolated fix.
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.