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

Feature: RTL Support #174

Open
wants to merge 11 commits into
base: v2.2
Choose a base branch
from

Conversation

itsmohmans
Copy link
Contributor

@itsmohmans itsmohmans commented Feb 22, 2023

This closes #173.

Changes:

  • Switch the CSS direction to 'RTL' when the current language is Arabic
  • Fixed the styling issues for RTL, including
    • Switching the right arrow icons to left, and vice versa
    • Switching the right margins to left, and vice versa
    • Setting the flex direction to row-reverse where needed

Please note that if any other RTL language added in the future, you'll have to edit this computed function

isRTL: function () {
     return this.$i18n.locale === 'ar'
}

in 4 places: App.vue, linkList.vue, configModal.vue, and tipsModal.vue, to make it check for other RTL languages as well. Currently, it's just Arabic.

@netlify
Copy link

netlify bot commented Feb 22, 2023

Deploy Preview for weektodo-app ready!

Name Link
🔨 Latest commit d710263
🔍 Latest deploy log https://app.netlify.com/sites/weektodo-app/deploys/63f6900e1ca1f10008bf7df6
😎 Deploy Preview https://deploy-preview-174--weektodo-app.netlify.app/
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@manuelernestog manuelernestog changed the base branch from main to v2.1 May 31, 2023 16:27
@manuelernestog
Copy link
Owner

Hi @itsmohmans. Thanks for the PR I didn't know about the RTL in apps.

I was reviewing your code and looks ok but when I see the app looks a little buggier, and it's normal because all the margins, paddings, when you move the calendar the movement, etc are designed for the opposite direction.

I'm not familiarized with countries that use RTL, so I have to ask if it would be better to see the user interface like this or have the UI in the standard way without the problems with the elements. My concern is that to make this looks good I't will take a lot of changes, conditionals and a lot of computed methods in the components just for this and I don't like to abuse if it can of this kind of methods because they impact in the performance of the app.

image

@manuelernestog
Copy link
Owner

manuelernestog commented May 31, 2023

Hey I was reviewing this PR again and my eyes are getting used to it a bit. What happened is that it didn't look very good because we still had to add styles to many things in the view.

I told you before that I was worried about putting the computed methods in the app but this can be avoided if we create a general rule and we can move all the styles to a specific file for the RTL rules

I'm making some changes directly to this branch to move to that format and I am applying the style of some elements.

Here what I have some doubts with the calendar, the arrows and the displacements

@itsmohmans
Copy link
Contributor Author

itsmohmans commented May 31, 2023

Hey @manuelernestog, I just took a quick look at it again, and I find it weird that the screenshot in your comment looks a bit different from the Netlify preview or the one I have locally (what I've noticed so far in the screenshot is that the arrow icons are not flipped correctly). But I do agree that there are some bugs I just noticed them, and I'll work on a quick fix for them soon.

Regarding the RTL layout, there are some rules to follow to make a correct RTL layout like of course using an RTL text direction and flipping margins / paddings accordingly, and some icons should be flipped (like arrows). Also in our case, the calendar view should be flipped, meaning that a day on the right should be before a day to its left, and the week view controls should be updated accordingly (this is the buggy part that should be fixed). You can read more about designing an RTL layout here in this blog post

Edit: yep it's a better idea to put all the RTL styles in one place, this would make it more clean and maintainable.

@manuelernestog
Copy link
Owner

Hi @itsmohmans. Excellent!!

Well, I already made some changes in this branch.

  • The branch was merged with v2.1 which have a lot of new changes in packages and dependencies, and in the visual aspect the more different is the config menu which was redesigned.
  • I already created the scss file with the RTL styles and move some of the one you already created and added some more.

So I think that the best should be pull this branch and work over it.

@manuelernestog manuelernestog changed the base branch from v2.1 to v2.2 February 9, 2024 03:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RTL support
2 participants