Skip to content
This repository was archived by the owner on Jan 13, 2025. It is now read-only.

docs(rtl): Simplify README to follow best practices #917

Merged
merged 7 commits into from
Jul 20, 2017
Merged

docs(rtl): Simplify README to follow best practices #917

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
da04472
docs(rtl): Simplify README to follow best practices
lynnmercier Jul 7, 2017
0ac8ba7
improve README and some fixes to _mixins.scss
lynnmercier Jul 18, 2017
28afaf2
Merge branch 'master' into docs/rtl
lynnmercier Jul 18, 2017
b41e56e
add paragraph about how each rtl mixin builds on the previous
lynnmercier Jul 18, 2017
1c599be
Merge branch 'master' into docs/rtl
lynnmercier Jul 19, 2017
0571a94
Merge branch 'master' into docs/rtl
lynnmercier Jul 20, 2017
5c52971
Merge branch 'master' into docs/rtl
lynnmercier Jul 20, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
207 changes: 12 additions & 195 deletions packages/mdc-rtl/README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@ path: /catalog/rtl/

# RTL

MDC RTL provides sass mixins to assist with RTL / bi-directional layouts within MDC-Web components.
Because we would like to achieve a standard approach to RTL throughout MDC-Web, we highly recommend
that any MDC-Web component that needs RTL support leverage this package.
UIs for languages that are read from right-to-left (RTL), such as Arabic and Hebrew, should be mirrored to ensure content is easy to understand.

## Design & API Documentation

Expand All @@ -28,76 +26,20 @@ npm install --save @material/rtl

## Usage

Simply `@import "@material/rtl/mixins";` and start using the mixins. Each mixin is described below.
### Sass Mixins

### mdc-rtl
`$base-property` is a base box-model property. e.g. margin / border / padding.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Listing the args out here looks a bit unnatural to me. Can we put it as part of the following table and use a more explanatory name, such as position-property?

`$pos` is a horizontal position property, either "left" or "right".
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should also update here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry...but I think maybe you're look at an old commit. I changed this in the latest commit.

Or maybe I screwed something up in my commits...


```scss
@mixin mdc-rtl($root-selector: null)
```

Creates a rule that will be applied when an MDC-Web component is within the context of an RTL layout.

Usage Example:

```scss
.mdc-foo {
position: absolute;
left: 0;

@include mdc-rtl {
left: auto;
right: 0;
}
| Mixin | Description |
| ----------------------------------------------- | - |
| `mdc-rtl($root-selector)` | Creates a rule that is applied when the root element is within an RTL context |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Create rules applied within $root-selector scope when the root element is contained within an RTL context

| `mdc-rtl-reflexive-box($base-property, $default-direction, $value, $root-selector)` | Applies the value to the `#{$base-property}-#{$default-direction}` property in a LTR context, and flips the direction in an RTL context. This mixin zeros out the original value in an RTL context. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

find a typo in comments here in mdc-rtl/mixin.scss line 128:

* You can also pass a 4th optional $root-selector argument which will be forwarded to `mdc-rtl`,
* e.g. `@include mdc-rtl-reflexive-box-property(margin, left, 8px, ".mdc-component")`.

It should be @include mdc-rtl-reflexive-box, we don't have a mixin called mdc-rtl-reflexive-box-property

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest we emphasize the size effect this mixin have, it zeros out the counter-party value, which might surprise our user in some case.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Try to emphasize this point :) WDYT?

| `mdc-rtl-reflexive-property($base-property, $left-value, $right-value, $root-selector)` | Emits rules that assign `#{$base-property}`-left to `#{left-value}` and `#{base-property}`-right to `#{right-value}` in a LTR context, and vice versa in a RTL context. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put a line when we should use mdc-rtl-reflexive-property instead of mdc-rtl-reflexive-box by a quick comparison between its difference?

| `mdc-rtl-reflexive-position($pos, $value, $root-selector)` | Applies the value to the specified position in a LTR context, and flips the direction in an RTL context. |
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

and here.


Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

IMHO, having a short example will help me understand the behavior of the mixin classes better than have only text. If you feel it is too long, I suggest we change the verbose example to a simpler one.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I want developers to read about what each mixin accomplishes. This lets developers decide where they can use the mixins in their own code. Once a developer is using a mixin, we can upgrade the implementation of the mixin so long as it still accomplishes the same original goal.

When we document code examples of the actual CSS output of the mixin, we risk developers using a mixin simply because they want that exact output...NOT because they want to accomplish the same goal the mixin aims to solve.

So we can document examples of where to use this mixins, but we should avoid documenting the actual CSS output. The CSS output is an implementation detail.

...I looked at our code and I couldn't actually find great examples of us using these properties...Theres one in checkbox, but it's more confusing to me than helpful.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pointed well taken, I agree that we shouldn't include transpiled css here. However, the single line document confuse me when to use which mixin class. Could we consider having a paragraph like:

In case for symmetric RTL properties, you can use mdc-rtl-reflexive-box. For example, when button have margin-left: 8px; margin-right: 0px; to left edge and margin-left: 0px; margin-right: 8px to right edge in RTL mode.

@include mdc-rtl-reflexive-box('margin', 'left', 8px, '.mdc-foo')

&__bar {
margin-left: 4px;
@include mdc-rtl(".mdc-foo") {
margin-left: auto;
margin-right: 4px;
}
}
}

.mdc-foo--mod {
padding-left: 4px;

@include mdc-rtl {
padding-left: auto;
padding-right: 4px;
}
}
```

will emit the following css:

```css
.mdc-foo {
position: absolute;
left: 0;
}
[dir="rtl"] .mdc-foo, .mdc-foo[dir="rtl"] {
left: auto;
right: 0;
}
.mdc-foo__bar {
margin-left: 4px;
}
[dir="rtl"] .mdc-foo .mdc-foo__bar, .mdc-foo[dir="rtl"] .mdc-foo__bar {
margin-left: auto;
margin-right: 4px;
}

.mdc-foo--mod {
padding-left: 4px;
}
[dir="rtl"] .mdc-foo--mod, .mdc-foo--mod[dir="rtl"] {
padding-left: auto;
padding-right: 4px;
}
```
**N.B.**: checking for `[dir="rtl"]` on an ancestor element works in most cases, it will sometimes
lead to false negatives for more complex layouts, e.g.
**A note about [dir="rtl"]**: `mdc-rtl($root-selector)` checks for `[dir="rtl"]` on the ancestor element. This works in most cases, it will sometimes lead to false negatives for more complex layouts, e.g.

```html
<html dir="rtl">
Expand All @@ -108,129 +50,4 @@ lead to false negatives for more complex layouts, e.g.
</html>
```

Unfortunately, we've found that this is the best we can do for now. In the future, selectors such
as [:dir](http://mdn.io/:dir) will help us mitigate this.

### mdc-rtl-reflexive-box

```scss
@mixin mdc-rtl-reflexive-box($base-property, $default-direction, $value, $root-selector: null)
```

Takes a base box-model property - e.g. margin / border / padding - along with a default
direction and value, and emits rules which apply the value to the
`#{$base-property}-#{$default-direction}` property by default, but flips the direction
when within an RTL context.

For example:

```scss
.mdc-foo {
@include mdc-rtl-reflexive-box(margin, left, 8px);
}
```
is equivalent to:

```scss
.mdc-foo {
margin-left: 8px;

@include mdc-rtl {
margin-right: 8px;
margin-left: 0;
}
}
```

Whereas:

```scss
.mdc-foo {
@include mdc-rtl-reflexive-box(margin, right, 8px);
}
```
is equivalent to:

```scss
.mdc-foo {
margin-right: 8px;

@include mdc-rtl {
margin-right: 0;
margin-left: 8px;
}
}
```

You can also pass a 4th optional $root-selector argument which will be forwarded to `mdc-rtl`,
e.g. `@include mdc-rtl-reflexive-box-property(margin, left, 8px, ".mdc-component")`.

Note that this function will always zero out the original value in an RTL context. If you're
trying to flip the values, use `mdc-rtl-reflexive-property`.

### mdc-rtl-reflexive-property

```scss
@mixin mdc-rtl-reflexive-property($base-property, $left-value, $right-value, $root-selector: null)
```

Takes a base property and emits rules that assign `#{$base-property}`-left to `#{left-value}` and
`#{base-property}`-right to `#{right-value}` in a LTR context, and vice versa in a RTL context.

For example:

```scss
.mdc-foo {
@include mdc-rtl-reflexive-property(margin, auto, 12px);
}
```
is equivalent to:

```scss
.mdc-foo {
margin-left: auto;
margin-right: 12px;

@include mdc-rtl {
margin-left: 12px;
margin-right: auto;
}
}
```

A 4th optional $root-selector argument can be given, which will be passed to `mdc-rtl`.

### mdc-rtl-reflexive-position

```scss
@mixin mdc-rtl-reflexive-position($pos, $value, $root-selector: null)
```

Takes an argument specifying a horizontal position property (either "left" or "right") as well
as a value, and applies that value to the specified position in a LTR context, and flips it in a
RTL context.

For example:

```scss
.mdc-foo {
@include mdc-rtl-reflexive-position(left, 0);
position: absolute;
}
```
is equivalent to:

```scss
.mdc-foo {
position: absolute;
left: 0;
right: initial;

@include mdc-rtl {
right: 0;
left: initial;
}
}
```

An optional third `$root-selector` argument may also be given, which is passed to `mdc-rtl`.
Unfortunately, we've found that this is the best we can do for now. In the future, selectors such as [:dir](http://mdn.io/:dir) will help us mitigate this.