a11y: right-to-left improvements #6235
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
github-actions
bot
added
automerge
♻️ autoupdate
| <div> | ||
| <Label htmlFor="username">{t("username")}</Label> | ||
| </div> | ||
| <div className="mt-2 flex rounded-md"> | ||
| <span | ||
| className={classNames( | ||
| "inline-flex items-center rounded-l-md border border-r-0 border-gray-300 bg-gray-50 px-3 text-sm text-gray-500" | ||
| )}> | ||
| {process.env.NEXT_PUBLIC_WEBSITE_URL.replace("https://", "").replace("http://", "")}/ | ||
| </span> | ||
| <div className="relative w-full"> | ||
| <Input | ||
| <TextField | ||
| ref={usernameRef} | ||
| name="username" | ||
| value={inputUsernameValue} | ||
| addOnLeading={ | ||
| <>{process.env.NEXT_PUBLIC_WEBSITE_URL.replace("https://", "").replace("http://", "")}/</> | ||
| } |
There was a problem hiding this comment.
replaced Input with Textfield
this needs to be tested
PeerRich
removed
♻️ autoupdate
|
@Udit-takkar can you test both rtl and ltr on the vercel preview? also worth having the current main branch on another browser tab to compare. hopefully ltr is identical. if not I made a mistake |
github-actions
bot
added
automerge
♻️ autoupdate
|
If we want to start doing this I think it's good if we communicate with everyone that they should take this into account. Otherwise we'll soon have a situation where this partially works, but breaks for new functionality. What do you think? Besides that, Tailwind will soon (finally 😬) add support for I know you've already done a bunch of work looking at the PR, but maybe it's worth it to wait for this change to get merged soon? |
There was a problem hiding this comment.
Checked the code, apart from 2 small questions that looks good.
So if we decide to go with this, instead of wait for tailwindlabs/tailwindcss#10166, then only Udit needs to test the website on preview and we're good to go in my opinion :)
cc @PeerRich
| @@ -282,7 +282,7 @@ function UserDropdown({ small }: { small?: boolean }) { | |||
| </span> | |||
| </span> | |||
| <Icon.FiMoreVertical | |||
| className="h-4 w-4 flex-shrink-0 text-gray-400 group-hover:text-gray-500 rtl:mr-2" | |||
| className="h-4 w-4 flex-shrink-0 text-gray-400 group-hover:text-gray-500 ltr:mr-2 rtl:ml-2 rtl:mr-4" | |||
There was a problem hiding this comment.
Is it correct that on ltr it only has a margin on right side, and on rtl it has a margin on both sides?
| @@ -50,15 +50,17 @@ const VerticalTabItem = function ({ | |||
| props.textClassNames || "text-sm font-medium leading-none text-gray-600", | |||
| "min-h-9 group flex w-64 flex-row items-center rounded-md px-2 py-[10px] hover:bg-gray-100 group-hover:text-gray-700 [&[aria-current='page']]:bg-gray-200 [&[aria-current='page']]:text-gray-900", | |||
| props.disabled && "pointer-events-none !opacity-30", | |||
| (isChild || !props.icon) && "ml-7 mr-5 w-auto", | |||
| (isChild || !props.icon) && "ml-7 w-auto ltr:mr-5 rtl:ml-5", | |||
There was a problem hiding this comment.
There's still a ml here, should that be removed?
This reverts commit 45d10a3.
to test, change dir="rtl"
CleanShot.2023-01-02.at.15.26.06.mp4