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.

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

Docs/list/new readme #2033

Merged
merged 9 commits into from
Jan 23, 2018
Merged

Docs/list/new readme #2033

merged 9 commits into from
Jan 23, 2018

Conversation

moog16
Copy link
Contributor

@moog16 moog16 commented Jan 22, 2018

@codecov-io
Copy link

codecov-io commented Jan 22, 2018

Codecov Report

Merging #2033 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2033      +/-   ##
==========================================
- Coverage   99.43%   99.43%   -0.01%     
==========================================
  Files          84       84              
  Lines        3718     3717       -1     
  Branches      485      485              
==========================================
- Hits         3697     3696       -1     
  Misses         21       21
Impacted Files Coverage Δ
packages/mdc-textfield/label/foundation.js 100% <0%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 836c8fa...6602543. Read the comment docs.

@@ -1,49 +1,23 @@
<!--docs:
Copy link
Contributor

Choose a reason for hiding this comment

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

Was this removed by accident? I thought we need it as front matter for publishing this on material.io?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah yes, you're right. Thanks for catching.

# Lists

<!--<div class="article__asset">
Copy link
Contributor

Choose a reason for hiding this comment

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

Same with these comments (see the template)

"A single continuous column of tessellated subdivisions of equal width." Both single-line and two-line lists are
supported (with three-line lists [planned](https://github.com/material-components/material-components-web/issues/31)).
MDC Lists are designed to be accessible and RTL aware.
MDC List provides styles which implement Material Design Lists - "A single continuous column of tessellated subdivisions of equal width." Both single-line and two-line lists are supported (with three-line lists planned). MDC Lists are designed to be accessible and RTL aware.
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe for the first line:

MDC List provides styles which implement a single continuous column of tessellated subdivisions of equal width.

<a href="https://material-components-web.appspot.com/list.html">Demo</a>
</li>
</ul>
* [Material Design guidelines: List](https://material.io/guidelines/components/lists.html)
Copy link
Contributor

Choose a reason for hiding this comment

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

This should also follow the template


A basic list consists simply of the list itself, and list items taking up one line.
## Single-Line List
Copy link
Contributor

Choose a reason for hiding this comment

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

I think for the Usage section, we should separate the sections first by lanaguage (HTML, then CSS/Sass) and then by type of list (single-line, two-line, etc). Then we can consolidate the CSS/Sass into a single reference table, instead having a CSS table for every type of list. E.g.

### HTML Structure
#### Single-line list
...
#### Two-line list
...
#### List groups
..
#### List dividers
...
### CSS Classes
...
### Sass Mixins
...

See the Text Field README for an example of this.

The reason for this is because the first thing developers want to know is what types of lists we provide and the HTML structure that they can copy/paste to use it, which is why we should put all of that at the beginning. Then, if they're interested in understanding the structure or CSS behind it, they can look at the CSS/Sass reference tables. There's also a lot of overlap in the CSS tables (mdc-list-item appears in multiple tables, for example) so we can consolidate all the tables into one, to be as concise as possible.

But that's my thinking, what are your thoughts on this?

Copy link
Contributor

Choose a reason for hiding this comment

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

Also maybe we should add a one-line description for all the different types of lists (single-line, two-line, etc).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yas, definitely. That makes sense to me. As someone new to MDC I would want all the classes in one place. I think the dev will be smart enough to figure out where the class should go.

And yes I'll add a one-liner for each type of list

Copy link
Contributor

@bonniezhou bonniezhou left a comment

Choose a reason for hiding this comment

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

Nice!! A few small changes/nits but otherwise LGTM!

some extra markup around the text to style a list in the two-line list style as defined by
[the spec](https://material.io/guidelines/components/lists.html#lists-specs) (see "Two-line lists").
#### Two-Line List
While in theory you can add any number of "lines" to a list item, you can use the mdc-list--two-line combined with some extra markup around the text to style a list in the two-line list style as defined by [the spec](https://material.io/guidelines/components/lists.html#lists-specs) (see "Two-line lists").
Copy link
Contributor

Choose a reason for hiding this comment

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

Style as inline code, i.e. mdc-list--two-line


To use within lists, simply add the `mdc-list-divider` class to a list item.
#### List Items
List-items (rows) can contain primary and secondary actions. Lists-items can contain 1 supporting graphic tile and/or 1 metadata tile that are positioned at the start and end of the list item, respectively.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: List items don't need to be hyphenated

element. `mdc-list-divider` elements can be used in these groups _between_ lists to help
differentiate them.
#### List Groups
Multiple related lists can be grouped together using the mdc-list-group class on a containing element.
Copy link
Contributor

Choose a reason for hiding this comment

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

Style as inline code, i.e. mdc-list-group

often in web applications, especially those suited more for desktop. The following example shows how
to add borders to lists.
#### List Dividers
MDC List contains an mdc-list-divider class which can be used as full-width or inset subdivisions either within lists themselves, or standalone between related groups of content.
Copy link
Contributor

Choose a reason for hiding this comment

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

Style as inline code, mdc-list-divider

</ul>
```

#### Control Tile Positions
> Note the role="separator" attribute on the list divider. It is important to include this so that assistive technology can be made aware that this is a presentational element and is not meant to be included as an item in a list. Note that separator is indeed a valid role for li elements.
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Style the note here similar to notes in the Text Field readme e.g.

NOTE: add note here

`mdc-list-divider--padded` | Optional, leaves gaps on each side of divider to match padding of list-item__meta
`mdc-list-divider--inset` | Optional, increases the leading margin of the divider so that it does not intersect the avatar column

\* **Note**: `mdc-list-divider` class can be used between list-items (example 1) *OR* between two lists (example 2)
Copy link
Contributor

Choose a reason for hiding this comment

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

See comment above about styling notes

* *Activated* state is similar to selected state, however should only be implemented once within a specific list.
* *Activated* state is more permanent than selected state, and will **NOT** change soon relative to the lifetime of the page.

### Sass Variables and Mixins
Copy link
Contributor

Choose a reason for hiding this comment

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

Remove "Variables" since there aren't any here

subdivisions either within lists themselves, or standalone between related groups of content.

To use within lists, simply add the `mdc-list-divider` class to a list item.
#### List Items
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm inclined to remove this section and just put the paragraph describing list items under Single-line lists...but it's up to you

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No I think you're right. the item is part of the list, and this section is quite bare

Copy link
Contributor Author

Choose a reason for hiding this comment

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

less is more

@moog16 moog16 merged commit dbac1e4 into master Jan 23, 2018
@moog16 moog16 deleted the docs/list/new-readme branch January 23, 2018 20:14
moog16 pushed a commit that referenced this pull request Jan 23, 2018
williamernest added a commit that referenced this pull request Jan 23, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants