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 3.0.0 side menu issue in RTL #11805

Closed
jafaripur opened this issue May 26, 2017 · 2 comments · Fixed by #11635
Closed

Ionic 3.0.0 side menu issue in RTL #11805

jafaripur opened this issue May 26, 2017 · 2 comments · Fixed by #11635

Comments

@jafaripur
Copy link

Ionic version: (check one with "x")
[ ] 1.x (For Ionic 1.x issues, please use https://github.com/driftyco/ionic-v1)
[ ] 2.x
[X] 3.x

I'm submitting a ... (check one with "x")
[X] bug report
[ ] feature request
[ ] support request => Please do not submit support requests here, use one of these channels: https://forum.ionicframework.com/ or http://ionicworldwide.herokuapp.com/

Current behavior:
After upgrade to version 3.0.0, Sidebar Menu have error in RTL mode with side="left" option.

Before this version this is work correctly. I think the padding, margin which added in last versuib generate this issue.

rtl: add rtl margin, padding, position and border-radius (#11342) (a30379b)

global packages:

    @ionic/cli-utils : 1.1.2
    Cordova CLI      : 7.0.1 
    Ionic CLI        : 3.1.2

local packages:

    @ionic/app-scripts              : 1.3.7
    @ionic/cli-plugin-cordova       : 1.2.1
    @ionic/cli-plugin-ionic-angular : 1.2.0
    Cordova Platforms               : android 6.2.3
    Ionic Framework                 : ionic-angular 3.3.0

System:

    Node       : v6.10.2
    OS         : Linux 4.4
    Xcode      : not installed
    ios-deploy : not installed
    ios-sim    : not installed
@AmitMY
Copy link
Contributor

AmitMY commented May 26, 2017 •
edited
Loading

That is correct.
Due to some limitations in that PR, I had to place LTR first and RTL second, and so I overridden LTR by doing right: auto - for this case.
To fix this, there is a version with a seperation of LTR and RTL - #11635
Along with a full fix for the menu side animation and gesture - #11336

We try not to, but allow to break RTL functionality for now, with the purpose of having full functionality in the correct way soon.

Here is a quick fix for you for now - for menu on the right side - you will be able to fix it for left as well, no problem.

[dir="rtl"] ion-menu[side=right] > .menu-inner {
    right: inherit;
    left: 0;
}

@AmitMY AmitMY mentioned this issue Jun 4, 2017
Merged
brandyscarney pushed a commit that referenced this issue Jun 6, 2017
@AmitMY @brandyscarney
feat(rtl): optimize the new mixins for smaller bundle, ltr separation (…
f0c6948 
…#11635)

* refactor(item): replaced item-left with item-start
replaced item-right with item-end

* style(item): fix spacing

* fix(item): add backwards support for left/right in ng-content

* fix(item): deprecated old variables, not breaking change

* feat(rtl): padding mixin

* feat(rtl): change all padding variables to start/end
add support for old variable names

* feat(rtl): replace all padding-side with start/end

* revert(functions): remove mixins

* feat(scss): add padding-horizontal and rtl functions (thanks brandy)

* feat(padding): use padding horizontal mixin everywhere

* feat(padding): use padding horizontal mixin everywhere

* fix(lint): change properties order. tests passing

* fix(sass-functions): reorder functions to avoid warning

* fix(scss): fix variable name

* perf(rtl): add check if need rtl selector

* feat(scss): add full padding function

* feat(scss): add border-radius mixin

* fix(rtl): change border-radius to use mixin

* perf(scss): only override if has something to override

* feat(scss): add margin scss variables for sides

* feat(scss): add margin mixin

* fix(scss): fix wrong support for 2/3 args

* feat(rtl): spread margins/paddings

* feat(rtl): spread margins/paddings

* feat(position): add rtl support for absolute

* fix(rtl): add missing calls

* fix(item): old attributes deprecated support

* revert(changelog): not intended to be changed

* fix(sass-functions): and not &&

* fix(padding): merge + missing padding

* style(): remove newline

* refactor(mixins): move mixins to mixins file

* style(): fix alignment

* fix(item): right padding should not be set

* fix(): incorrect defaults

* feat(scss-lint): disable some side variables

* fix(scss): lint passes

* feat(lint): disabled text-align

* fix(): correct variable name

* fix(fab): missed a comma

* fix(rtl): rtl method incorrect for multiple selectors

* fix(rtl): toolbar bad merge

* fix(rtl): icon-only is in px not em

* fix(rtl): toggle padding

* feat(rtl): correct notation for rtl custom

* Merge branch 'breaking-item' into start-end

# Conflicts:
#	src/components/checkbox/checkbox.ios.scss
#	src/components/checkbox/checkbox.md.scss
#	src/components/checkbox/checkbox.wp.scss
#	src/components/item/item.ios.scss
#	src/components/item/item.md.scss
#	src/components/item/item.wp.scss
#	src/components/radio/radio.ios.scss
#	src/components/radio/radio.md.scss
#	src/components/radio/radio.wp.scss
#	src/components/toggle/toggle.ios.scss
#	src/components/toggle/toggle.md.scss
#	src/components/toggle/toggle.wp.scss

* feat(rtl): optimize bundle result

* feat(float): use new standard

* feat(platform): gotta have direction on html

* fix(scss): add import + change code order

* fix(lint): passes

* fix(scss): fix for deprecated usages

* fix(scss): property use #{}

* fix(rtl): change css specificity, increase bundle size for multidirectional

* fix(scss): mixin manages priority

* fix(scss): select icon

* fix(scss): correct range variable

* fix(lint): unused import

* fix(multi-dir): things that ignore PropertySpelling must be under multi-dir

* fix(multi-dir): things that ignore PropertySpelling must be under multi-dir

* fix(include-rtl): change to app-direction, to be able to set rtl only apps

* fix(rtl): remove incorrect split-pane rtl behavior

* fix(menu): start needs position on rtl

* fix(float): renamed test file, add float mixin

* fix(scss-lint): lint excluded files

* fix(scss): add missing unit

* refactor(scss): change dir to direction, as the correct property name

* fix(app-direction): replace last usage of include-rtl

Closes: #11805 #11914
@ionitron-bot
Copy link

ionitron-bot bot commented Sep 2, 2018

Thanks for the issue! This issue is being locked to prevent comments that are not relevant to the original issue. If this is still an issue with the latest version of Ionic, please create a new issue and ensure the template is fully filled out.

@ionitron-bot ionitron-bot bot locked and limited conversation to collaborators Sep 2, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

feat(rtl): optimize the new mixins for smaller bundle, ltr seperation AmitMY/ionic
3 participants
@jafaripur @AmitMY @brandyscarney