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(fab): Update README to match best practices #1252

Merged
merged 5 commits into from
Sep 8, 2017
Merged

Conversation

lynnmercier
Copy link
Contributor

No description provided.

@lynnmercier
Copy link
Contributor Author

Please wait for #1241 before merging

@codecov-io
Copy link

codecov-io commented Sep 7, 2017

Codecov Report

Merging #1252 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1252   +/-   ##
======================================
  Coverage    99.9%   99.9%           
======================================
  Files          69      69           
  Lines        3307    3307           
  Branches      407     407           
======================================
  Hits         3304    3304           
  Misses          3       3

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 b146aa9...ae483f8. Read the comment docs.

`mdc-fab` | Mandatory, for the button element
`mdc-fab__icon` | Mandatory, for the icon element
`mdc-fab--mini` | Optional, modifies the FAB to a smaller size
`mdc-fab--exited` | Optional, animates the FAB out of view.<br>When this class is removed, the FAB will return to view.
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for already factoring this in here. ☀️

<span class="mdc-fab__icon">
favorite
foo_icon
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there a reason not to use a valid material-icons value here? (We also still reference favorite in the absolute-positioning example.)

</span>
</button>
```

### Absolutely positioned
### CSS Classes
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it normal for this to be as far removed from the Sass sections of the readme as it is? (e.g. should we consider moving the Absolutely Positioned and Adding MDC Ripple sections further down below the Sass stuff?)

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 think users will use HTML and CSS. Then they will probably also want to position their FAB. They will probably also want to add a Ripple, though maybe not if JS is too heavy. The least likely thing they want to do is customize the FAB's color (least likely after the positioning and ripple). So I've sorted the features to the top which I think most users will want.

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.

None yet

3 participants