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(breadcrumbs): add RTL support to the breadcrumbs component (#2486) #2487

Closed
wants to merge 2 commits into from

Conversation

mrbadri
Copy link
Contributor

@mrbadri mrbadri commented Mar 9, 2024

Closes #2486

📝 Description

Add RTL support to the breadcrumbs component.

⛳️ Current behavior (updates)

he breadcrumbs component currently lacks support for right-to-left (RTL) direction.

🚀 New behavior

This PR addresses the RTL support issue in the breadcrumbs component, ensuring correct rendering in RTL environments.

💣 Is this a breaking change (Yes/No):

📝 Additional Information

@mrbadri mrbadri requested a review from jrgarciadev as a code owner March 9, 2024 14:32
Copy link

changeset-bot bot commented Mar 9, 2024

🦋 Changeset detected

Latest commit: e120f30

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
@nextui-org/breadcrumbs Patch
@nextui-org/react Patch

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

Copy link

vercel bot commented Mar 9, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
nextui-storybook-v2 ✅ Ready (Inspect) Visit Preview 💬 Add feedback Mar 10, 2024 10:07am

Copy link

vercel bot commented Mar 9, 2024

@mrbadri is attempting to deploy a commit to the NextUI Inc Team on Vercel.

A member of the Team first needs to authorize it.

Copy link
Contributor

@alphaxek alphaxek left a comment

Choose a reason for hiding this comment

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

@mrbadri this looks fine.
is it possible to fix tooltip as well, in case of rtl tooltip still shows on the left side.

ref:
tooltip

@mrbadri
Copy link
Contributor Author

mrbadri commented Mar 10, 2024

@mrbadri this looks fine. is it possible to fix tooltip as well, in case of rtl tooltip still shows on the left side.

Hi alphaxek,

I have resolved this issue with commit e120f30.

return (
  <NextUIProvider locale={locale}>
    <div className="bg-dark" lang={locale} dir={direction}>
      <Story />
    </div>
  </NextUIProvider>
  // Modal component renders here
  // Tooltips component renders here
);

The direction in the root div does not affect the tooltip and modal, so we must change the direction in the body CSS.

@alphaxek
Copy link
Contributor

alphaxek commented Mar 10, 2024

@mrbadri thanks!
As per my analysis tooltip's div gets generated outside root div.

Ref:
image

@@ -14,7 +14,11 @@ const Breadcrumbs = forwardRef<"div", BreadcrumbsProps>((props, ref) => {
children,
childCount,
itemProps,
separator = <ChevronRightIcon />,
separator = (
<div className="rtl:transform rtl:rotate-180">
Copy link
Member

Choose a reason for hiding this comment

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

@mrbadri all the tailwindcss styles should be in the theme package, this is because the users load the styles only from that folder https://nextui.org/docs/guide/installation#tailwind-css-setup so all tailwindcss classes out of there will not be compiled

@wingkwong wingkwong changed the base branch from main to canary April 28, 2024 14:54
@tareq96
Copy link

tareq96 commented Aug 12, 2024

@wingkwong any update on this? when it's expected to go live?

@wingkwong
Copy link
Member

Closing - inactivity.

@wingkwong wingkwong closed this Aug 31, 2024
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] - Lack of RTL Support in breadcrumbs
5 participants