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

RTL support and Arabic Translations, closes #978 #1067

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nLoops
Copy link

@nLoops nLoops commented Apr 18, 2020

Please follow this checklist. Please check each appropriate box (put an 'x' or check it after creating the PR).

  • REQUIRED: Do you have an Issue assigned to you within the v1 milestone for this PR? Put the Issue number here:
  • Provided detailed pull request description and a succinct title (consider template below for guidance).
  • Tested your changes, especially after any code review iterations.
  • Included any relevant screenshots of UI updates.
  • Followed the Contributor Guidelines.
  • Verified all contributions are properly licensed pursuant to the LICENSE file in the root of the repository.
  • Verified your name is in the content/credits.yaml file (if you want it to be).

After all boxes above are checked, request and receive an Approved review from any team member knowledgable in the area (TODO team member list). Once approved, the team member will assign your review to a Committer or use the needs-merge label.

What does this PR accomplish?

Added the Arabic translation ARB file, also improve layouts behavior with RTL languages by making the appbar back-arrow points to the right and adding some margin space to the right.

issue #978

PS. there're some parts of texts not covered by the ARB files, and it will be provided by LINGO24.

Did you add any dependencies?

No.

How did you test the change?

Android device screenshots

flutter_01

flutter_02

flutter_03

flutter_04

flutter_05

@hspinks hspinks added client Mobile client/app component i18n Internationalization l10n Localization (translations) string-change PR changes strings, will need to be translated labels Apr 18, 2020
@kassim
Copy link
Contributor

kassim commented Apr 18, 2020

from the homepage screenshot - shouldn't the alignment be to the right? also there's an ampersand (also on the questions page)

);
Widget build(BuildContext context) => Transform.rotate(
// back arrow need to point right in the RTL languages.
angle: Constants.isDirectionRTL(context) ? 180 * pi/180 : 0,
Copy link
Contributor

Choose a reason for hiding this comment

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

Ideally we would mirror rather than rotate.

Copy link
Author

Choose a reason for hiding this comment

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

Yup, in this case, we need another SVG data for the right arrow.

Copy link
Contributor

@kassim kassim Apr 20, 2020

Choose a reason for hiding this comment

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

You can rotate around the Y axis using Transform(transform :Matrix4.rotationY(180)...)
see https://stackoverflow.com/questions/58047009/flutter-how-to-flip-an-icon-to-get-mirror-effect

Copy link
Author

Choose a reason for hiding this comment

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

I will give it a try, but I think it's the same behavior.

@advayDev1 advayDev1 changed the title Rtl/arabic #978 RTL support and Arabic Translations, closes #978 Apr 20, 2020
@advayDev1 advayDev1 added the needs:triage New issue that needs triage label May 26, 2020
@stale
Copy link

stale bot commented Jun 25, 2020

This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the resolved:stale No recent activity on the issue or PR label Jun 25, 2020
@stale
Copy link

stale bot commented Jul 2, 2020

This item has been automatically closed as stale.

@stale stale bot closed this Jul 2, 2020
@brunobowden brunobowden reopened this Feb 9, 2021
@stale stale bot removed the resolved:stale No recent activity on the issue or PR label Feb 9, 2021
@brunobowden
Copy link
Collaborator

@nLoops - thanks for your work on this before. Now that the app is launched, it would be great to get your RTL work landed. My hope is that there's isn't too much change to resolve the merge conflicts.

We're about to kick off translation, so RTL is clearly important for that. I'm sure there's some other work that will be needed too on localization, so it would be very helpful to have your help on that. Please connect with me on the Slack #localization channel. Email me at [email protected] if you need an invite.

@brunobowden
Copy link
Collaborator

@nLoops - FYI, the strings have probably changed quite a bit since you do the original translation.... but we'll figure that out too.

@stale
Copy link

stale bot commented Feb 16, 2021

This item has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the resolved:stale No recent activity on the issue or PR label Feb 16, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
client Mobile client/app component i18n Internationalization l10n Localization (translations) needs:triage New issue that needs triage resolved:stale No recent activity on the issue or PR string-change PR changes strings, will need to be translated
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants