-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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(docs): replace twitter logo/links with x logo/links #3815
Conversation
🦋 Changeset detectedLatest commit: 1ee8df9 The changes in this PR will be included in the next version bump. This PR includes changesets to release 0 packagesWhen changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
@dgz9 is attempting to deploy a commit to the NextUI Inc Team on Vercel. A member of the Team first needs to authorize it. |
WalkthroughThe changes in this pull request primarily involve updating URLs from the old Twitter domain to the new X domain across various files in the repository. This includes modifications to contact links, icon components, and documentation, ensuring that all references to Twitter are replaced with the corresponding X links. Additionally, a new icon component for X has been introduced, and existing components have been updated to utilize this new icon. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Warning Review ran into problems🔥 ProblemsError running Biome: Command failed with exit code 4: jk_cp -o -f -k -j /cache/46/4c91c798-6b79-4bfd-9f5d-34e61379e032 /dev/null /dev/urandom /usr/bin/ast-grep /usr/bin/curl /usr/bin/diff /usr/bin/dirname /usr/bin/env /usr/bin/fd /usr/bin/gawk /usr/bin/gh /usr/bin/jq /usr/bin/patch /usr/bin/rg /usr/bin/shellcheck /usr/bin/tokei /usr/bin/tree /usr/bin/uniq /usr/bin/xargs /etc/ssl/certs /etc/passwd /etc/group /home/jailuser/ast-grep-rules ERROR: /usr/bin/ast-grep does not exist Usage: /usr/sbin/jk_cp -j [OPTIONS] -h --help : this help screen Note: if no jail is specified, the first argument is Error running Gitleaks: Cloning the repository previously failed. Error running semgrep: Cloning the repository previously failed. Git: Failed to clone repository. Please run the 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
|
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: 0
🧹 Outside diff range and nitpick comments (6)
apps/docs/content/components/user/link-description.ts (1)
8-8
: LGTM! Consider updating the link text for consistency.The URL update from Twitter to X is correct and aligns with the PR objectives. Well done!
For complete consistency with the rebranding, consider updating the link text from "@jrgarciadev" to "x.com/jrgarciadev". Here's a suggested change:
- <Link href="https://x.com/jrgarciadev" size="sm" isExternal> - @jrgarciadev + <Link href="https://x.com/jrgarciadev" size="sm" isExternal> + x.com/jrgarciadev </Link>This change would fully reflect the transition from Twitter to X in both the URL and the displayed text.
apps/docs/libs/constants.tsx (1)
3-3
: Consider updating the constant name for consistencyWhile the value of
TWITTER_USER_NAME
is still correct (as it refers to the username), consider updating the constant name itself for consistency with the rebranding. For example, you could rename it toX_USER_NAME
.Here's a suggested change:
-export const TWITTER_USER_NAME = "getnextui"; +export const X_USER_NAME = "getnextui";Don't forget to update any references to this constant throughout the codebase if you make this change.
apps/docs/components/navbar.tsx (1)
333-333
: Update aria-label and verify link properties.The icon has been correctly updated to use
XIcon
. However, consider the following suggestions:
- Update the
aria-label
from "Twitter" to "X" for consistency.- Verify if
siteConfig.links.twitter
needs to be updated to reflect the new X.com domain.Consider applying these changes:
<Link isExternal - aria-label="Twitter" + aria-label="X" className="p-1" href={siteConfig.links.twitter} onPress={() => handlePressNavbarItem("Twitter", siteConfig.links.twitter)} > <XIcon className="text-default-600 dark:text-default-500" /> </Link>Also, verify if
siteConfig.links.twitter
and thehandlePressNavbarItem
parameters need to be updated to use "X" instead of "Twitter".apps/docs/content/blog/v2.3.0.mdx (1)
Line range hint
212-217
: LGTM: Calendar presets section addedThe new section about Calendar presets is informative and highlights a useful feature. However, the mentioned code example is missing.
Consider adding the code example for the Calendar presets to provide a complete understanding of the feature:
<CodeDemo title="Calendar Presets" files={calendarContent.presets} />apps/docs/components/icons/social.tsx (2)
469-469
: Export updated correctly, but consider TwitterIcon usage.The XIcon has been correctly added to the export statement. However, both TwitterIcon and XIcon are now exported. Consider if TwitterIcon should be deprecated or removed to maintain consistency with the rebranding to X.
If TwitterIcon is to be deprecated, consider adding a comment or console warning when TwitterIcon is used:
export const TwitterIcon: React.FC<IconSvgProps> = (props) => { console.warn('TwitterIcon is deprecated. Please use XIcon instead.'); return XIcon(props); };
Multiple Twitter References Found. Please Update to X.
Several Twitter mentions remain in the codebase, including constants, configurations, sponsor details, and component usages. To ensure a complete rebranding from Twitter to X, please review and update all remaining references.
- Update constants and configuration values referencing Twitter.
- Modify sponsor information where Twitter handles are used.
- Replace
TwitterIcon
withXIcon
in all components.- Adjust any URLs pointing to
twitter.com
to their corresponding X URLs.🔗 Analysis chain
Line range hint
1-469
: Verify other Twitter references in the codebase.The XIcon has been successfully added and exported. To ensure complete rebranding from Twitter to X, it's important to check for other Twitter references throughout the codebase.
Run the following script to find potential Twitter references:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for Twitter references in the codebase # Test: Search for 'Twitter' or 'twitter' in all files rg -i 'twitter' --type-not svg # Test: Search for twitter.com URLs rg 'https?://(?:www\.)?twitter\.com' # Test: Search for TwitterIcon usage rg 'TwitterIcon'Length of output: 3228
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (15)
- .github/ISSUE_TEMPLATE/config.yml (1 hunks)
- apps/docs/.sponsorsrc (3 hunks)
- apps/docs/components/icons/social.tsx (2 hunks)
- apps/docs/components/navbar.tsx (2 hunks)
- apps/docs/config/site.ts (1 hunks)
- apps/docs/content/blog/nextui-v2.mdx (2 hunks)
- apps/docs/content/blog/v2.1.0.mdx (2 hunks)
- apps/docs/content/blog/v2.2.0.mdx (1 hunks)
- apps/docs/content/blog/v2.3.0.mdx (6 hunks)
- apps/docs/content/components/checkbox-group/custom-styles.ts (1 hunks)
- apps/docs/content/components/checkbox/custom-styles.ts (1 hunks)
- apps/docs/content/components/user/link-description.ts (1 hunks)
- apps/docs/libs/constants.tsx (2 hunks)
- packages/components/user/stories/user.stories.tsx (1 hunks)
- packages/core/react/README.md (1 hunks)
✅ Files skipped from review due to trivial changes (8)
- .github/ISSUE_TEMPLATE/config.yml
- apps/docs/.sponsorsrc
- apps/docs/config/site.ts
- apps/docs/content/blog/v2.1.0.mdx
- apps/docs/content/blog/v2.2.0.mdx
- apps/docs/content/components/checkbox-group/custom-styles.ts
- apps/docs/content/components/checkbox/custom-styles.ts
- packages/core/react/README.md
🧰 Additional context used
🪛 LanguageTool
apps/docs/content/blog/nextui-v2.mdx
[uncategorized] ~94-~94: Possible missing comma found.
Context: ...nents already include theuse client;
directive so you can import them directly in your...(AI_HYDRA_LEO_MISSING_COMMA)
🔇 Additional comments (10)
apps/docs/libs/constants.tsx (3)
1-1
: LGTM: Import statement updated correctlyThe import statement has been properly updated to use
XIcon
instead ofTwitterIcon
, which aligns with the PR objective of updating Twitter references to X.
11-11
: LGTM: URL updated correctlyThe URL has been properly updated from "https://twitter.com/getnextui" to "https://x.com/getnextui", which aligns with the PR objective of updating Twitter links to X links. The username (getnextui) has been correctly maintained.
10-10
: Icon component updated, but consider visual consistencyThe icon component has been correctly updated to use
XIcon
. However, there are two points to consider:
- The color has been changed from Twitter blue (#00ACEE) to white (#ffffff). Ensure this doesn't affect visibility on different backgrounds.
- The icon size has been reduced from 32 to 28. Verify that this doesn't disrupt visual balance with other icons in the UI.
To check for consistency, run the following script:
✅ Verification successful
Icon component update verified
After reviewing the icon sizes and colors across the codebase, the changes to the
XIcon
(color changed to #ffffff and size reduced to 28) align with the visual consistency of other icons.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for icon sizes and colors in the codebase # Search for icon size definitions echo "Icon sizes:" rg --type typescript --type javascript 'size=\{(\d+)\}' # Search for icon color definitions echo "\nIcon colors:" rg --type typescript --type javascript 'className="[^"]*text-\[[^\]]+\]'Length of output: 274
Script:
#!/bin/bash # Description: Check for icon sizes and colors in the codebase # Search for icon size definitions echo "Icon sizes:" rg -g '*.ts' -g '*.tsx' 'size=\{(\d+)\}' # Search for icon color definitions echo "\nIcon colors:" rg -g '*.ts' -g '*.tsx' 'className="[^"]*text-\[[^\]]+\]"'Length of output: 9170
packages/components/user/stories/user.stories.tsx (1)
61-61
: LGTM: URL updated correctly.The Twitter URL has been successfully updated to the new X domain. This change aligns with the PR objective and correctly reflects the platform's rebranding.
Let's verify if there are any other Twitter-related references in the file:
✅ Verification successful
Verified: All Twitter references updated correctly.
No remaining Twitter-related references found in the file, and only the expected
.com
domains are present. The change aligns with the PR objective to update URLs from Twitter to X.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining Twitter-related references in the file. # Test: Search for 'twitter' or 'tweet' (case-insensitive). Expect: No matches. rg -i 'twitter|tweet' packages/components/user/stories/user.stories.tsx # Test: Search for any remaining '.com' domains. Expect: Only 'x.com' and 'github.com'. rg '\.com' packages/components/user/stories/user.stories.tsxLength of output: 260
apps/docs/components/navbar.tsx (1)
37-37
: LGTM: Import statement updated correctly.The import statement has been successfully updated to use
XIcon
instead ofTwitterIcon
, which aligns with the PR objective of replacing Twitter references with X.apps/docs/content/blog/v2.3.0.mdx (4)
10-10
: LGTM: Author link updated to X (formerly Twitter)The author's link has been correctly updated from "twitter.com" to "x.com", aligning with the rebranding of Twitter to X.
Line range hint
302-341
: LGTM: Removal of units creation section addedThe new section about removing the
units
creation is well-explained and provides clear upgrade instructions. This information is crucial for users updating to the new version.
Line range hint
342-362
: LGTM: Separation of errorMessage and isInvalid section addedThe new section about separating
errorMessage
andisInvalid
is well-explained and provides clear upgrade instructions. This information is crucial for users updating to the new version and understanding the new validation approach.
Line range hint
384-406
: LGTM: Additional improvements and changes documentedThe new items in the "Improvements" section provide valuable information about the removal of custom
units
creation and updates to react versions. These additions align well with the breaking changes mentioned earlier in the document.apps/docs/components/icons/social.tsx (1)
72-81
: LGTM! New XIcon component added.The new XIcon component has been implemented correctly and consistently with other icon components. Note that the default size is set to 20, while other icons typically use 24. This might be intentional for the X logo, but it's worth considering for consistency across icons.
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
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.
TwitterIcon
can be removed since it's not in use.
Will do tonight. Do you want me to remove all references of Twitter and replace with X or just keep it at the logo and links? |
I mean just |
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: 0
🧹 Outside diff range and nitpick comments (1)
.changeset/slimy-cats-brush.md (1)
1-6
: LGTM! Minor suggestion for clarity.The changeset file accurately reflects the updates made in this PR. It correctly identifies the packages being updated and provides a clear description of the changes related to the Twitter/X rebranding.
Consider slightly modifying the description for added clarity:
- Update Twitter logo, links, and named references to X. Removed Twitter logo from social based on review feedback. + Update Twitter logo, links, and named references to X. Removed Twitter logo from social icons based on review feedback.This small change specifies that the Twitter logo was removed from the social icons, which aligns more precisely with the changes made in the PR.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (6)
- .changeset/slimy-cats-brush.md (1 hunks)
- .github/ISSUE_TEMPLATE/config.yml (1 hunks)
- apps/docs/components/icons/social.tsx (2 hunks)
- apps/docs/components/navbar.tsx (2 hunks)
- apps/docs/libs/constants.tsx (1 hunks)
- packages/core/react/README.md (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- .github/ISSUE_TEMPLATE/config.yml
- apps/docs/components/navbar.tsx
- apps/docs/libs/constants.tsx
- packages/core/react/README.md
🔇 Additional comments (2)
apps/docs/components/icons/social.tsx (2)
Line range hint
61-69
: LGTM! New XIcon component added correctly.The new
XIcon
component is well-structured and consistent with other icon components. Note that the default size is set to 20, which differs from the typical default of 24 used in other icons. This might be intentional for the X logo, but it's worth confirming.
457-457
: Export statement updated correctly.The export statement has been properly updated to include the new
XIcon
and remove theTwitterIcon
. This change aligns with the PR objectives and the discussed removal of unused components.
I was wondering about all the references to twitter, but I left it for now. I removed the |
Is it ready for review or you still have further changes? |
Ready for review. Let me know if you want any other changes. |
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.
final round of changes
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.
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: 0
🧹 Outside diff range and nitpick comments (1)
apps/docs/components/marketing/features-grid.tsx (1)
75-77
: Approve changes with a minor suggestion for consistencyThe update to the
LinkIcon
className improves visibility in both light and dark modes, which is a good enhancement. The icon will now be black in light mode and white in dark mode, ensuring better contrast against the background.For consistency with other components in the file, consider using the
slots
object for styling. You could add a new slot for the link icon and use it like this:-<LinkIcon className="text-black dark:text-white" height={18} width={18} /> +<LinkIcon className={slots.linkIcon({class: classNames?.linkIcon})} height={18} width={18} />Then, add the following to the
styles
object at the top of the file:linkIcon: "text-black dark:text-white",This approach would maintain consistency with the styling method used throughout the component.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- apps/docs/components/marketing/features-grid.tsx (1 hunks)
- apps/docs/libs/constants.tsx (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/docs/libs/constants.tsx
Fixed light theme icon. Also noticed the feature grid icon was not working for light more, changed that too. |
Closes # N/A
📝 Description
⛳️ Current behavior (updates)
🚀 New behavior
💣 Is this a breaking change (Yes/No):
📝 Additional Information
Summary by CodeRabbit
New Features
XIcon
component for the updated branding.Bug Fixes
Documentation
Style
LinkIcon
in theFeaturesGrid
component by removing the specific class name.