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.

Sign up for GitHub

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

Jump to bottom

Ionic 4.0.0 RTL issues #17012

Closed
abennouna opened this issue Jan 9, 2019 · 131 comments
Closed

Ionic 4.0.0 RTL issues #17012

abennouna opened this issue Jan 9, 2019 · 131 comments
Assignees
@abennouna @brandyscarney
Labels
package: core @ionic/core package type: bug a confirmed bug report

Comments

@abennouna
Copy link
Contributor

abennouna commented Jan 9, 2019 •
edited by liamdebeasi
Loading

This issue is to collect and keep track of RTL bugs vs. master so we can make sure to properly update and fix them.

Below are:

  • a list of some utilities presenting issues in RC.0,
  • and a list of components with tests included in the test suite.

Legend

Symbol Description
✅ Tested. Found compliant.
☑️ Tested. Initially not compliant or Regression. Solved by PR specified in description.
❓ Not necessarily an issue, or not a core-specific issue, or design decision.
⚠️ Not tested. Help needed.
❌ Tested. Not completely compliant. Should list the defects.

Fixed issues

CSS Utilities

Grid

Components

Existing Issues as of 4.4.1

  • item-divider
    • ❌ slots margins in MD: in both slot="start" and slot="end", a small 2px margin appears on one side in LTR, but not on the opposite side in RTL (see https://i.imgur.com/4W5TFs2.png for an illustration of the problem: the box model shows the slotted ion-button))

See also: animation is not rtl-aware: #19489

Contributing

Please help with the untested components, or double-check the tested ones if you feel like it. Thank you for the contribution anyway!

If you're not familiar with the included test suite:

  1. Fork the Ionic repo;
  2. Use the master branch;
  3. Setup your local copy;
  4. Run the test suite locally;
  5. When you navigate to the component you want to test, you can add ?rtl=true to the url in order to check the RTL version;
  6. Submit a comment with the issue.

See the contributing documentation for more information.

You can also test your own app using @ionic/angular@dev
@ionitron-bot ionitron-bot bot added the triage label Jan 9, 2019
@paulstelzer paulstelzer added package: core @ionic/core package type: bug a confirmed bug report and removed triage labels Jan 9, 2019
This was referenced Jan 9, 2019
Closed
Closed
Closed
@brandyscarney brandyscarney mentioned this issue Jan 9, 2019
Closed
11 tasks
@Newbie012
Copy link

Scroller inside <ion-select> on RTL isn't positioned correctly.
Demo - https://gyazo.com/d1602f045193380be6d02432a7e61878

@abennouna
Copy link
Contributor Author

abennouna commented Jan 10, 2019 •
edited
Loading

Yes indeed, but I had a hard time debugging it to understand what's going on there... I'll just add it to the list then

@abennouna
Copy link
Contributor Author

@Newbie012 I noticed it's only the case with the AlertController interface, and in MD. Can you please verify?

@Newbie012
Copy link

@abennouna True

@abennouna
Copy link
Contributor Author

Thanks @Newbie012, added to the list under the alert component

@brandyscarney brandyscarney pinned this issue Jan 11, 2019
@brandyscarney
Copy link
Member

I updated the original issue to include links to some of the open RTL issues. We can either keep those open for tracking or close them to track here.

@twinssbc
Copy link

@abennouna I notice that dir option is removed form ion-slides in v4 which is supposed to RTL when using slides component. May I know will it be added back or is there any alternative way to achieve it?

@abennouna
Copy link
Contributor Author

abennouna commented Jan 13, 2019 •
edited
Loading

@twinssbc slides work in RTL mode out of the box as long as the document is in RTL. Are you referring to something else? PS: There is an issue though if you want to switch direction dynamically: you'd have to reload the document to keep swiper events working correctly

@MuhAssar
Copy link

@abennouna I would like to thank you for your effort in making sure that all RTL issues are fixed.
Jazakallahu Khairan

I just want to mention that items inside ion-item-divider in RTL has different margins with both slot="start" and slot="end" than in LTR

@abennouna
Copy link
Contributor Author

@abuassar 🤝don't mention it!

Good call! I see a 2px margin that disappears in RTL mode, only in MD. Can you confirm please?

@abennouna abennouna changed the title [WIP] Ionic 4 RC RTL issues Ionic 4 RC RTL issues Jan 13, 2019
@MuhAssar
Copy link

MuhAssar commented Jan 14, 2019 •
edited
Loading

@abuassar 🤝don't mention it!

Good call! I see a 2px margin that disappears in RTL mode, only in MD. Can you confirm please?

It is not just 2px error, please check the following screen shots:

START uses slot="start"
END uses slot="end"
MIDDLE is without a slot

RTL
ionic4-rtl

LTR
ionic4-ltr

and here is how to recreate the issue
app.component.html:

https://gist.github.com/abuassar/bdee78a37e36cf1c5e7ddc506eed141d

@MuhAssar
Copy link

unfortunately I found the same issue with ion-item:

START uses slot="start"
END uses slot="end"
MIDDLE is without a slot

ion-item-issue

demo:
app.component.html
https://gist.github.com/abuassar/d957e7b6d5079cd7f30550c91b7c4617

@abennouna
Copy link
Contributor Author

@abuassar am I right to assume you're not using the latest master with PR #16987? I updated the issue description to explain the test context. Here's the current render of the issue in item-divider: https://i.imgur.com/4W5TFs2.png

@MuhAssar
Copy link

@abennouna you are right, I'm using rc1 as I thought it has the fix.

Would you please guide me how to use latest master with the said PR?

@abennouna
Copy link
Contributor Author

@abuassar I updated the issue description with the following in the "Contributing" part:

If you're not familiar with the included test suite:

  1. Fork the Ionic repo;
  2. Use the rtl branch (or use the master branch and merge the PR fix(rtl): enable ltr default, rtl selectors #16987);
  3. Setup your local copy;
  4. Run the test suite locally;
  5. When you navigate to the component you want to test, you can add ?rtl=true to the url in order to check the RTL version;
  6. Submit a comment with the issue.

Please let me know if it's unclear.

@twinssbc
Copy link

@twinssbc slides work in RTL mode out of the box as long as the document is in RTL. Are you referring to something else? PS: There is an issue though if you want to switch direction dynamically: you'd have to reload the document to keep swiper events working correctly

@abennouna I can confirm that slides work in the default RTL mode, no extra option is required.

brandyscarney added a commit that referenced this issue Jan 15, 2019
@brandyscarney
fix(alert): use the proper borders for the buttons for rtl
92de1b7 
references #17012
@brandyscarney brandyscarney mentioned this issue Jan 15, 2019
Merged
6 tasks
@tmwrnr tmwrnr mentioned this issue Aug 2, 2019
Closed
@3adeling
Copy link

3adeling commented Aug 4, 2019 •
edited
Loading

@abennouna

this issue: #17012 (comment)

still exist in real ios device/simulator but not in browser !

ionic version: 4.7.1

@3adeling
Copy link

3adeling commented Aug 6, 2019

this is happening because of the following style:

:host( .fab-horizontal-center) {
margin-left: unset;
}

removing unset will solve the problem but I am not sure how to override/cancel it
I tried all other values for margin but they didn't work
only removing it will fix the issue!

@mrahmadt
Copy link

mrahmadt commented Aug 6, 2019 via email

@3adeling
Copy link

3adeling commented Aug 7, 2019

thanks @mrahmadt but that didn't work

the solution is:

in global.scss

.fab-horizontal-center {
    margin-left: -28px !important;
}

also I notice that fab start position is not changing direction in RTL mode in ios simulator/device

@SherifMoShalaby
Copy link

ion-slides rtl is not working properly ?
any help

@yzedayyash
Copy link

Hello, i have a problem with side menu in real IOS devices
Its open from left to right in RTL mode!!
Can any one help me with that or any one has fixed this issue

@SherifMoShalaby
Copy link

@brandyscarney
any updates regarding ion-slides rtl

@3alampro
Copy link

any update on the RTL support for ion-slides if I changed document dir to RTL the ion-slide stop sweeping between slides

@SherifMoShalaby
Copy link

any update on the RTL support for ion-slides if I changed document dir to RTL the ion-slide stop sweeping between slides

It swipes but in the wrong direction

@chriswep
Copy link

@abennouna i still see the issue "select icon in iOS: fix position of the inner element" on the current version, although its supposed to be fixed according to the issue description.

@brandyscarney brandyscarney pinned this issue May 13, 2020
@liamdebeasi
Copy link
Contributor

@abennouna It looks like columns in ion-picker do not switch position based upon LTR vs RTL mode: #21205.

Previous work seems to indicate this is intentional:

datetime
☑️ picker in iOS/MD: fix start/end columns’ horizontal positioning when there are 2 or 3 picker columns (PR #18339)
☑️ with floating or stacked label: the datetime text should stay in the document flow (regression) (PR #18340)
✅ picker in iOS/MD: keep the order of columns as in LTR when using time and/or DD/MM/YYYY date format (see #16294) (PR: #17018)

Do you recall the reasoning behind this?

@nourkrimesh
Copy link

nourkrimesh commented Jun 14, 2020 •
edited
Loading

Hello Everyone,
I discovered some issues with ion-item-sliding component in RTL (Ionic 5):
First:
the ion-item-options doesn't open correctly especially in iOS
I fixed it with css:
ion-item-options {
width: unset !important;
}
it fixed the problem and the option appears correct in both LTR and RTL

Second:
when I'm trying to open it programmatically with .open('end') method, it opened in LTR but with RTL it activating the class named: item-sliding-active-options-start but the side is end so it doesn't open:
To fix it I added a second ion-item-options with side="start" and after opening it and do what I want I'm removing it. It's not a good solution but It solve my problem right now.

@REPTILEHAUS REPTILEHAUS mentioned this issue Jun 24, 2020
Closed
@3adeling
Copy link

3adeling commented Jul 13, 2020 •
edited
Loading

Hey. I wonder if swipe to go back direction is already solved or not. Because the direction is not changing right now.

#19488

@3adeling
Copy link

<ion-list> detail icon is showing wrong direction when changing direction in the same page (without reloading) using: document.documentElement.setAttribute('dir', 'ltr');

@liamdebeasi liamdebeasi unpinned this issue Sep 18, 2020
@ChiragPrajapat
Copy link

Hello, i have a problem with side menu in real IOS devices
Its open from left to right in RTL mode!!
Can any one help me with that or any one has fixed this issue

this is not working for me too..

@shahramSo
Copy link

shahramSo commented Sep 25, 2020 •
edited
Loading

@ChiragPrajapat, I recently reported a bug for this issue and show a walkthrough about that, check it and hope, it is useful for you.
#22116

@timsar2
Copy link

timsar2 commented Sep 2, 2021 •
edited
Loading

ion-slides rtl is not working properly ?
any help

Late but i fix it by setting with of ion-sliding-options as unset

ion-item-options:not(.item-options-start){
        width: unset !important;
 }

wand1252 added a commit to wand1252/ionic-ons-Cordova that referenced this issue Aug 31, 2022
@wand1252
fix(icon): update to include rtl styles #656
38abbdf 
- adds the ability to run npm start and launch the icon component locally
- adds a test for viewing the icon component locally
- updates the component css to include rtl styles
- updates contributing to include steps for this

references ionic-team/ionic-framework#17012
@liamdebeasi liamdebeasi mentioned this issue Nov 17, 2022
Merged
13 tasks
@liamdebeasi liamdebeasi mentioned this issue Mar 20, 2023
Merged
13 tasks
This was referenced Mar 24, 2023
Closed
Merged
thetaPC added a commit that referenced this issue Apr 4, 2023
@thetaPC @liamdebeasi @Ionitron
fix(item-divider): removal of unneeded margin unset (#27042)
d925237 
## Pull request checklist

Please check if your PR fulfills the following requirements:
- [ ] Tests for the changes have been added (for bug fixes / features)
- [ ] Docs have been reviewed and added / updated if needed (for bug
fixes / features)
- Some docs updates need to be made in the `ionic-docs` repo, in a
separate PR. See the [contributing
guide](https://github.com/ionic-team/ionic-framework/blob/main/.github/CONTRIBUTING.md#modifying-documentation)
for details.
- [x] Build (`npm run build`) was run locally and any changes were
pushed
- [x] Lint (`npm run lint`) has passed locally and any fixes were made
for failures


## Pull request type

Please check the type of change your PR introduces:
- [x] Bugfix
- [ ] Feature
- [ ] Code style update (formatting, renaming)
- [ ] Refactoring (no functional changes, no api changes)
- [ ] Build related changes
- [ ] Documentation content changes
- [ ] Other (please describe): 


## What is the current behavior?

End slots are lacking margin of 2px when in RTL.

Issue URL: Part of #17012  


## What is the new behavior?

- Simplifying `property-horizontal` by removing any margin unset

End slots with buttons will have a margin-end of 2px regardless of
direction.

### Screenshots

![Screen Shot 2023-03-24 at 13 48
25](https://user-images.githubusercontent.com/13530427/227642440-acf0f2c2-37c7-43da-bad4-67ef1b31b3ac.png)
![Screen Shot 2023-03-24 at 13 48
17](https://user-images.githubusercontent.com/13530427/227642445-79cf2986-c0b5-45d1-ba38-1beda698c2d5.png)


## Does this introduce a breaking change?

- [ ] Yes
- [x] No



## Other information

- Continuation of #27024 in order to apply it
to Framework v7

---------

Co-authored-by: Liam DeBeasi <liamdebeasi@users.noreply.github.com>
Co-authored-by: ionitron <hi@ionicframework.com>
@thetaPC
Copy link
Contributor

thetaPC commented Apr 7, 2023 •
edited by liamdebeasi
Loading

Closing since the last known issue ("slots margins in MD") in the list was fixed and merged into main this week. Additionally the issue with "animation is not rtl-aware" was also fixed and merged.

Please use ^v7.0.1 for these fixes.

If related bugs are encountered, please open new issues for them.

@thetaPC thetaPC closed this as completed Apr 7, 2023
@ionic-team ionic-team locked as resolved and limited conversation to collaborators Apr 7, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@abennouna abennouna

@brandyscarney brandyscarney

Labels
package: core @ionic/core package type: bug a confirmed bug report
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests