-
Notifications
You must be signed in to change notification settings - Fork 6.8k
docs(expansion): create overview and examples #5823
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
Conversation
templateUrl: 'expansion-overview-example.html', | ||
}) | ||
export class ExpansionStepsExample { | ||
public step = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jelbourn - Do you prefer explicit public? I haven't seen this in other places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nope, in fact our coding standards specifically say to omit it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated the code ommiting 'public' keyword.
templateUrl: 'expansion-overview-example.html', | ||
}) | ||
export class ExpansionStepsExample { | ||
public step = 0; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Updated it.
src/lib/expansion/expansion.md
Outdated
|
||
Each expansion has a header and an action sections. The header section is always visible at the top of the component and contains a title and a description subsections. The action section is fixed at the bottom, being visible when the expansion is in expanded state | ||
|
||
When grouped by an `<md-accordion>` element, the expansions can be used for the creation of flows, as it brings up the possibility to expand one view at a time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The doc structure should be
- Short, one-sentence summary of the component
- Overview example
- Expanded description of the component
- Other component specific sections
- Accessibility section
src/lib/expansion/expansion.md
Outdated
|
||
### Accordion | ||
|
||
It's possible to group expansions in a fancy way. The `multi="true"` input allows the expansions state to be set independently of each other. When `multi="false"` (default) just one expansion can be expanded at a given time: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's possible to group expansions in a fancy way.
This isn't really a helpful sentence. Instead I would say something like "Multiple expansion panels can be combined into an accordion"
I've changed it. |
src/lib/expansion/expansion.md
Outdated
@@ -0,0 +1,81 @@ | |||
`<md-expansion-panel>` provides a way to show and hide lightweight content, by collapsing and expanding a view with a nice animation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
`<md-expansion-panel>` provides a expandable details-summary view.
Statements like "with a nice animation" are subjective. The docs should aim to be more "just the facts".
The word "lightweight" from the spec doesn't really have any bearing on the implementation. The designers used this word in the spec to communicate that people shouldn't dump an entire application inside an expansion panel.
src/lib/expansion/expansion.md
Outdated
|
||
The `expanded` input allows to choose among the collapsed or expanded state. | ||
|
||
Each expansion panel has a header and an action sections. The header section is always visible at the top of the component and contains a title and a description subsections. The action section is fixed at the bottom, being visible when the expansion is in expanded state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The header is required but the actions are optional
src/lib/expansion/expansion.md
Outdated
|
||
Each expansion panel has a header and an action sections. The header section is always visible at the top of the component and contains a title and a description subsections. The action section is fixed at the bottom, being visible when the expansion is in expanded state. | ||
|
||
When grouped by an `<md-accordion>` element the expansion panels can be used to create ordered views or flows, as it brings up the possibility to expand one view at a time. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would eliminate this sentence since we talk about accordions in their own section.
src/lib/expansion/expansion.md
Outdated
|
||
When grouped by an `<md-accordion>` element the expansion panels can be used to create ordered views or flows, as it brings up the possibility to expand one view at a time. | ||
|
||
### Events |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't need a section for events since this is captured in the API docs
src/lib/expansion/expansion.md
Outdated
|
||
<!-- example(expansion-overview) --> | ||
|
||
The `expanded` input allows to choose among the collapsed or expanded state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would omit this bit about the expanded
property because it will be captured via API docs and examples.
src/lib/expansion/expansion.md
Outdated
|
||
The `expanded` input allows to choose among the collapsed or expanded state. | ||
|
||
Each expansion panel has a header and an action sections. The header section is always visible at the top of the component and contains a title and a description subsections. The action section is fixed at the bottom, being visible when the expansion is in expanded state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would start a new section here like
### Expansion panel content
And then talk about the header, actions, title, and description.
width: 100%; | ||
text-align: right; | ||
margin-top: 5px; | ||
color: blue; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Too much indent (should be +2, not +4)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@julianobrasil I think you're missing an eof newline here (see Travis).
Also, I'm not 100% sure, but I think all css classes under material-examples folder, should be prefixed with example-
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done, @rafaelss95 .
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You forgot to change headers-align
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! Done.
Changed. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also I think the general preference in this repo is to break markdown text lines at 100 characters
src/lib/expansion/expansion.md
Outdated
|
||
The header section is always visible at the top of the component and contains a `<md-panel-title>` and a description `<md-panel-description>` subsections. | ||
|
||
The `<md-panel-title>` subsecion is shown in the begining of the header, followed by the `<md-panel-description>` subsection, which is supposed to contain a sumary of what's in the expansion content. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typos: subsecion -> subsection, begining -> beginning, sumary -> summary
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/lib/expansion/expansion.md
Outdated
|
||
The `<md-panel-title>` subsecion is shown in the begining of the header, followed by the `<md-panel-description>` subsection, which is supposed to contain a sumary of what's in the expansion content. | ||
|
||
By default, the expansion panel header has a toogle sign at the right edge, pointing up when the panel is expanded and down when it's collapsed. The toogle icon can be hidden by setting the input property `toogleHide` to `true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
toogle -> toggle (multiple)
"toggle sign" -> "toggle icon"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
src/lib/expansion/expansion.md
Outdated
|
||
#### Actions | ||
|
||
The actions section is optional and fixed at the bottom, being visible only when the expansion is in its expanded state. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can remove "being"
"...at the bottom, visible only when"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
@willshowell, manually (with simple |
just a FYI,
|
@mackelito, I noticed it. It wasn't like this. It's maybe something with the late build. BTW I moved the Edited: Apparently has the same root cause of #5623 and has a PR (#5650). |
src/lib/expansion/expansion.md
Outdated
@@ -0,0 +1,86 @@ | |||
`<md-expansion-panel>` provides a way to show and hide content, by collapsing and expanding a | |||
view with animation. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'd prefer to phrase this as
`<md-expansion-panel>` provides a expandable details-summary view.
src/lib/expansion/expansion.md
Outdated
#### Header | ||
|
||
The header section is always visible at the top of the component and contains a `<md-panel-title>` | ||
and a description `<md-panel-description>` subsections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would replace this paragraph and the next with
The `<md-expansion-panel-header>` shows a summary of the panel content and acts
as the control for expanding and collapsing. This header may optionally contain an
`<md-panel-title>` and an `<md-panel-description>`, which format the content of the
header to align with Material Design specifications.
src/lib/expansion/expansion.md
Outdated
|
||
By default, the expansion panel header has a toggle icon at the right edge, pointing up when | ||
the panel is expanded and down when it's collapsed. The toggle icon can be hidden by setting the | ||
input property `hideToggle` to `true`. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
By default, the expansion-panel header includes a toggle icon at the end of the
header to indicate the expansion state. This icon can be hidden via the
`hideToggle` property.
src/lib/expansion/expansion.md
Outdated
|
||
<!-- example(expansion-overview) --> | ||
|
||
### Expansion panel content |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expansion-panel
(here and elsewhere)
src/lib/expansion/expansion.md
Outdated
|
||
### Expansion panel content | ||
|
||
Each expansion panel has a header section (mandatory) and an actions (optional) sections. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Each expansion panel must include a header and may optionally include an action bar.
src/lib/expansion/expansion.md
Outdated
</md-panel-description> | ||
</md-expansion-panel-header> | ||
|
||
<p>...</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
<p>This is the primary content of the panel.</p>
src/lib/expansion/expansion.md
Outdated
|
||
#### Actions | ||
|
||
The actions section is optional and fixed at the bottom, visible only when the expansion is in its |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actions may optionally be included at the bottom of the panel
Done. |
@@ -0,0 +1,70 @@ | |||
<md-accordion class="example-headers-align"> | |||
<md-expansion-panel [expanded]="step === 0" (opened)="openEvent(0)" hideToggle="true"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Personally I think the toggles should be shown (instead of the icons) since this is the main example and you're trying to demo the features of the panel. If you insist on using icons,
- The
color: blue
is pretty harsh and could probably be dropped - The icon-to-right thing cuts off the description text. Instead, unwrap the icons from their parent div, and make
mat-expansion-panel-header-description
justify-content with space-between and align-items center
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icons are there just to show a situation where the the default is hidden (and to show that is possible to use some html inside the description). They are not really necessary. I can drop them. But before I'll try your suggestions to see how they look like.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Your html point is good... maybe wait for a team member before dropping? But yeah, I think the styles can be cleaned up a little anyway 😁
export class ExpansionStepsExample { | ||
step = 0; | ||
|
||
openEvent(stepNumb: number) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How about setStep(index: number)
?
https://github.com/angular/material2/blob/master/CODING_STANDARDS.md#methods
src/lib/expansion/expansion.md
Outdated
@@ -0,0 +1,87 @@ | |||
`<md-expansion-panel>` provides a expandable details-summary view. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
...an expandable...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@willshowell, done. Take a look.
Also, the example plunk: https://plnkr.co/edit/5q98WJ?p=preview
</md-input-container> | ||
|
||
<md-input-container> | ||
<input mdInput placeholder="Age"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
type="number"?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@Component({ | ||
selector: 'expansion-overview-example', | ||
templateUrl: 'expansion-overview-example.html', | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Component({
selector: 'expansion-steps-example',
templateUrl: 'expansion-steps-example.html',
styleUrls: ['expansion-steps-example.css'],
})
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added the missing styleUrls
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The template and selector need to change too! Looks like copy/paste from the other example
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The steps example was built over the overview one.
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Fix #5568