You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@airflow.apache.org by "guan404ming (via GitHub)" <gi...@apache.org> on 2025/06/03 17:17:45 UTC
[PR] Add rtl support [airflow]
guan404ming opened a new pull request, #51376:
URL: https://github.com/apache/airflow/pull/51376
<!--
Licensed to the Apache Software Foundation (ASF) under one
or more contributor license agreements. See the NOTICE file
distributed with this work for additional information
regarding copyright ownership. The ASF licenses this file
to you under the Apache License, Version 2.0 (the
"License"); you may not use this file except in compliance
with the License. You may obtain a copy of the License at
http://www.apache.org/licenses/LICENSE-2.0
Unless required by applicable law or agreed to in writing,
software distributed under the License is distributed on an
"AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
KIND, either express or implied. See the License for the
specific language governing permissions and limitations
under the License.
-->
<!--
Thank you for contributing! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
Feel free to ping committers for the review!
In case of an existing issue, reference it using one of the following:
closes: #ISSUE
related: #ISSUE
How to write a good git commit message:
http://chris.beams.io/posts/git-commit/
-->
## Related Issue
#51187
## Why
- to support rtl lang we need to add "dir" attribute in \<html\>
## How
- add it dynamically using i18n-next api hook
- determine the `dir` using [`i18n.dir()`](https://www.i18next.com/overview/api#dir)
- add `LocaleProvider` align to [official tutorial](https://www.chakra-ui.com/guides/overview-rtl-support) for chakra-ui
https://github.com/user-attachments/assets/4c6ac2a0-d2bd-4b26-8ed2-27ec9753c773
<!-- Please keep an empty line above the dashes. -->
---
**^ Add meaningful description above**
Read the **[Pull Request Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)** for more information.
In case of fundamental code changes, an Airflow Improvement Proposal ([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals)) is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a newsfragment file, named `{pr_number}.significant.rst` or `{issue_number}.significant.rst`, in [airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "Dev-iL (via GitHub)" <gi...@apache.org>.
Dev-iL commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955369614
> In general, I think that the menu should be on the right - that's how it works in Wikipedia, for example. Is it complicated to implement?
No objections here, it is the known convention after all.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955286676
> In general, I think that the menu should be on the right - that's how it works in Wikipedia, for example. Is it complicated to implement?
I agree
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "bbovenzi (via GitHub)" <gi...@apache.org>.
bbovenzi commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2941386414
I would like to defer to users like @Dev-iL if the would prefer the nav bar to be moved to the right side too.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955131130
Hi @eladkal @Dev-iL, after my investigation, the select issue is fixed by manual add style for RTL on `IndicatorGroup` which seems like the chakra-ui issue and I could help open a fix pr to chakra-ui.
I also upgrade our chakra-ui package from `3.17.0` to `3.20.0` version since it has [some rtl fixes](https://github.com/chakra-ui/chakra-ui/releases) which would reduce the manual fix above.
Please take another look and let me know if there is anything needing to modify. Current ui looks like
https://github.com/user-attachments/assets/27138410-4229-49f3-9db3-cf64bb5c87e9
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "shahar1 (via GitHub)" <gi...@apache.org>.
shahar1 commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955144759
> > I reviewed several pages and it looks good.
>
> Hi @eladkal Thanks for the review. I was missing a (more) clear statement if switching to RTL means that navbar also need to switch or if it is rather "better"/okay to keep it left. For me it is a bit tstrange to switch all to RTL but keeping the navbar left. But I am not an expert in this topic.
>
> ...or @shahar1 do you have an opinion?
In general, I think that the menu should be on the right - that's how it works in Wikipedia, for example.
Is it complicated to implement?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2949743880
I reviewed several pages and it looks good.
Most of the things that don't work properly are due to translations issues so I will address them
There is one thing with strings that start with numbers they are not aligned properly. The number should be first.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955445348
> You know, I actually prefer this for the English version. Any chance we can make the navbar side user-configurable and not language-specific?
Maybe we could open another issue for it to make sure if there are many people love this feature as well. I think it is not complex to implement but this change needs community's support~
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "Dev-iL (via GitHub)" <gi...@apache.org>.
Dev-iL commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955373581
> Sure, that makes sense. it's not complex to implement. It would look like
> <img alt="image" width="1440" src="https://private-user-images.githubusercontent.com/105915352/452969733-6c42d5d3-1c77-4e2c-9fbc-c80882680748.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDk0NjQ4ODMsIm5iZiI6MTc0OTQ2NDU4MywicGF0aCI6Ii8xMDU5MTUzNTIvNDUyOTY5NzMzLTZjNDJkNWQzLTFjNzctNGUyYy05ZmJjLWM4MDg4MjY4MDc0OC5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNjA5JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDYwOVQxMDIzMDNaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1iZGNjYmVlOTM4MzdjNmJkYjNlY2U5OGU0YTMxNWZmM2E2YTJiYzM5ZTVkZjgyZDMyMmE1OGEzNTE1Y2NhN2M1JlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.TupJ_ZOO01c3il8jMcUM8L8UNFOGONcEI8Apijf5WFw">
You know, i actually prefer this for the English version. Any chance we make the navbar side user-configurable and not language-specific?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal merged PR #51376:
URL: https://github.com/apache/airflow/pull/51376
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955369514
> In general, I think that the menu should be on the right - that's how it works in Wikipedia, for example.
Is it complicated to implement?
Sure, that makes sense. it's not complex to implement. It would look like
<img width="1440" alt="image" src="https://github.com/user-attachments/assets/6c42d5d3-1c77-4e2c-9fbc-c80882680748" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "eladkal (via GitHub)" <gi...@apache.org>.
eladkal commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2941481418
Thanks for raising this!
I will test this change combined with the Hebrew translation tommorow
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2945062737
> Personally, I hate it when GUIs are mirrored and I stay away from HE software for that reason alone.
I've noticed the issue of the nav bar and I also saw some comments above. Thus, I think we should listen to native speakers' opinions to decide whether we should also mirror it.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "pierrejeambrun (via GitHub)" <gi...@apache.org>.
pierrejeambrun commented on code in PR #51376:
URL: https://github.com/apache/airflow/pull/51376#discussion_r2127003678
##########
airflow-core/src/airflow/ui/index.html:
##########
@@ -1,5 +1,5 @@
<!doctype html>
-<html lang="en" style="height: 100%">
+<html style="height: 100%">
Review Comment:
nice
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "jscheffl (via GitHub)" <gi...@apache.org>.
jscheffl commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2955130599
> I reviewed several pages and it looks good.
Hi @eladkal Thanks for the review. I was missing a (more) clear statement if switching to RTL means that navbar also need to switch or if it is rather "better"/okay to keep it left. For me it is a bit tstrange to switch all to RTL but keeping the navbar left. But I am not an expert in this topic.
...or @shahar1 do you have an opinion?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2945089895
> Also some input have minor issues:
@pierrejeambrun I couldn't find or reproduce this issue on my local machine. Could you tell me more about the detail for this issue? I could diagnose and fix it. Thanks!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "guan404ming (via GitHub)" <gi...@apache.org>.
guan404ming commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2945268515
> In my picture above, some inputs do not have the `arrow down` icon that is mirrored properly.
>
> Some of them do, but some don't and therefore the text is overlapping with the carret down.
>
> Let me know if you also experience this.
I got it but it seems really strange. It would not overlap when I opened it with Chrome but it would be broken when I using Arc Browser to open it. The `arrow down` icon is not mirrored properly for both them. Diagnosing it currently.
Chrome
<img width="722" alt="image" src="https://github.com/user-attachments/assets/c27be71d-ab0b-447e-804a-d38ec3ca7c7e" />
Arc
<img width="732" alt="image" src="https://github.com/user-attachments/assets/fc84f1a4-56a0-487d-9c5b-d7b4d1b91030" />
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] Add RTL support [airflow]
Posted by "Dev-iL (via GitHub)" <gi...@apache.org>.
Dev-iL commented on PR #51376:
URL: https://github.com/apache/airflow/pull/51376#issuecomment-2945874058
> Chrome
> <img alt="image" width="722" src="https://private-user-images.githubusercontent.com/105915352/451982383-c27be71d-ab0b-447e-804a-d38ec3ca7c7e.png?jwt=eyJhbGciOiJIUzI1NiIsInR5cCI6IkpXVCJ9.eyJpc3MiOiJnaXRodWIuY29tIiwiYXVkIjoicmF3LmdpdGh1YnVzZXJjb250ZW50LmNvbSIsImtleSI6ImtleTUiLCJleHAiOjE3NDkxNTIyOTEsIm5iZiI6MTc0OTE1MTk5MSwicGF0aCI6Ii8xMDU5MTUzNTIvNDUxOTgyMzgzLWMyN2JlNzFkLWFiMGItNDQ3ZS04MDRhLWQzOGVjM2NhN2M3ZS5wbmc_WC1BbXotQWxnb3JpdGhtPUFXUzQtSE1BQy1TSEEyNTYmWC1BbXotQ3JlZGVudGlhbD1BS0lBVkNPRFlMU0E1M1BRSzRaQSUyRjIwMjUwNjA1JTJGdXMtZWFzdC0xJTJGczMlMkZhd3M0X3JlcXVlc3QmWC1BbXotRGF0ZT0yMDI1MDYwNVQxOTMzMTFaJlgtQW16LUV4cGlyZXM9MzAwJlgtQW16LVNpZ25hdHVyZT1jNDI1ZmU1MjEzM2VmZDNjYTEyZTI5M2RmNjQ5MTJiYTg2NzliZjI3YTdkM2MxZjA2NTMxODVhMTgzYzM4NzAwJlgtQW16LVNpZ25lZEhlYWRlcnM9aG9zdCJ9.rlwdO_V2K_1mycHggB9kg7nqIsLXEYxy9a1Lk8PjyZE">
Did you notice how "Meta Database" has the checkmark on the opposite side of the other tags?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org