-
-
Notifications
You must be signed in to change notification settings - Fork 179
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
base: v2.2
Are you sure you want to change the base?
Feature: RTL Support #174
Conversation
✅ Deploy Preview for weektodo-app ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
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. |
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 |
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. |
Hi @itsmohmans. Excellent!! Well, I already made some changes in this branch.
So I think that the best should be pull this branch and work over it. |
This closes #173.
Changes:
row-reverse
where neededPlease note that if any other RTL language added in the future, you'll have to edit this computed function
in 4 places:
App.vue
,linkList.vue
,configModal.vue
, andtipsModal.vue
, to make it check for other RTL languages as well. Currently, it's just Arabic.