feat(rtl): optimize the new mixins for smaller bundle, ltr seperation #11635
Conversation
replaced item-right with item-end
# Conflicts: # src/components/toggle/toggle.md.scss
add support for old variable names
# Conflicts: # src/components/button/button-icon.scss
…-optimize # Conflicts: # src/components/tabs/tabs.wp.scss
| @@ -97,16 +97,15 @@ $action-sheet-ios-button-cancel-font-weight: 600 !default; | |||
|
|
|||
| .action-sheet-ios .action-sheet-group { | |||
| @include border-radius($action-sheet-ios-border-radius); | |||
| @include margin(null, null, $action-sheet-ios-group-margin-bottom - 2, null); | |||
There was a problem hiding this comment.
Why do we need to use the mixin for margin-bottom, margin-top, padding-bottom, or padding-top? These shouldn't be affected by RTL, no?
There was a problem hiding this comment.
In theory, you are correct, and that is what I've done before. In practice no.
span {
margin-top: 1px;
}
[dir=rtl] span {
margin: 5px;
}Gives the span margin: 5px, because of specificity.
[dir=rtl] span {
margin-top: 1px;
}
[dir=rtl] span {
margin: 5px;
}Gives the span margin: 1px 5px 5px 5px, because of specificity. (which is the wanted behavior)
Don't worry, it gets super squashed at the end
| @@ -20,10 +20,6 @@ $split-pane-side-max-width: 28% !default; | |||
| contain: strict; | |||
| } | |||
|
|
|||
| [dir="rtl"] .split-pane { | |||
| flex-direction: row-reverse; | |||
There was a problem hiding this comment.
because it is not the correct behavior.. I checked on an iPad, in the mail app, the menu part is on the right on rtl, and left on ltr
| @@ -82,6 +82,15 @@ | |||
|
|
|||
| // --------------------------------------------------------------------------------- | |||
|
|
|||
| // Minimum breakpoint width. Null for the smallest (first) breakpoint. | |||
There was a problem hiding this comment.
Probably incorrect to do this in this PR, but the order of the mixins is giving IDE errors, as we call an undefined method
src/themes/ionic.mixins.scss
Outdated
| @mixin multi-dir() { | ||
| @if $app-direction == null { | ||
| $root: #{&}; | ||
| @at-root [dir="ltr"], [dir="rtl"] { |
There was a problem hiding this comment.
Why do we need to specify dir if the rule applies to both? Also will this work without @at-root being in front of [dir="rtl"]?
There was a problem hiding this comment.
Because of CSS specificity rules, otherwise it overrides stuff. And no, the @at-root seems to be a must
| @mixin ltr() { | ||
| @if $app-direction == null { | ||
| $root: #{&}; | ||
| @at-root [dir="ltr"] { |
There was a problem hiding this comment.
I'm not crazy about forcing a dir in the app.
Short description of what this resolves:
Changes proposed in this pull request:
$app-direction: nullis the default, which means all available directions.$app-direction: ltror$app-direction: rtlmake sure the app only support one direction (reducing bundle from 687kb to 575kb, using everything default)Ionic Version: 3.x
Fixes preview: http://ionic-snapshot-go.appspot.com/ionic2/snapshots/d4t/hhp/chrome_400x800
Closes: #11805 #11914