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.

Sign up for GitHub

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

Jump to bottom

aio + docs/api: pipe API pages #22702

Closed
wants to merge 3 commits into from
Closed

aio + docs/api: pipe API pages #22702

wants to merge 3 commits into from

Conversation

petebacondarwin
Copy link
Member

This change adds:

  • an impure badge for Pipes that are marked as pure: false
  • a pipe specific overview that shows the syntax for using a pipe in a template.
  • an "input value" section describing the type of the value that the pipe expects.
  • a "pipe params" section describing any additional params that a pipe expects.

@petebacondarwin petebacondarwin added action: review The PR is still awaiting reviews from at least one requested reviewer comp: aio target: major This PR is targeted for the next major release labels Mar 11, 2018
@mary-poppins
Copy link

You can preview 64276a2 at https://pr22702-64276a2.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 4982294 at https://pr22702-4982294.ngbuilds.io/.

@petebacondarwin
Copy link
Member Author

petebacondarwin commented Mar 11, 2018 •
edited

Here is how the DatePipe looks:

screen shot 2018-03-11 at 12 14 00

@petebacondarwin
Copy link
Member Author

Here is how the AsyncPipe looks:

screen shot 2018-03-11 at 12 14 11

@petebacondarwin petebacondarwin added this to REVIEW in docs-infra Mar 11, 2018
@IgorMinar IgorMinar mentioned this pull request Mar 12, 2018
Closed
34 tasks
gkalpak
gkalpak approved these changes Mar 12, 2018
View reviewed changes
aio/tools/transforms/templates/api/includes/pipe-overview.html Outdated

<section class="{$ doc.docType $}-overview">
<code-example hideCopy="true" class="no-box api-heading">{{ {$ doc.valueParam.name $}_expression | <span class="kwd nocode">{$ doc.pipeName $}</span>
{%- for param in doc.pipeParams %} : {% if param.isOptional or param.defaultValue !== undefined %}[ {% endif -%}
Copy link
Member

Choose a reason for hiding this comment

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

Should the [ be placed before the :?

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm, I guess so.

aio/tools/transforms/angular-api-package/processors/extractPipeParams.spec.js Outdated
{ name: 'transform', parameterDocs: [valueParam, pipeParam1, pipeParam2] }
] } ];
processor.$process(docs);
expect(docs[0].valueParam).toEqual(valueParam)
Copy link
Member

Choose a reason for hiding this comment

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

With toEqual you aren't really testing that it extracted the first element as valueParam.
You should either use toBe() or make the three objects different.

aio/tools/transforms/angular-api-package/processors/extractPipeParams.spec.js Outdated
{ name: 'transform', parameterDocs: [valueParam, pipeParam1, pipeParam2] }
] } ];
processor.$process(docs);
expect(docs[0].pipeParams).toEqual([pipeParam1, pipeParam2]);
Copy link
Member

Choose a reason for hiding this comment

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

Again toEqual() doesn't guarantee that the correct objects have been extracted (since all objects are identical).

@gkalpak gkalpak added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 12, 2018
@petebacondarwin petebacondarwin force-pushed the aio-api-pipes branch 2 times, most recently from 9238378 to 12f1c6c Compare March 14, 2018 09:20
@mary-poppins
Copy link

You can preview 9238378 at https://pr22702-9238378.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 12f1c6c at https://pr22702-12f1c6c.ngbuilds.io/.

@petebacondarwin petebacondarwin added action: merge The PR is ready for merge by the caretaker and removed action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews labels Mar 14, 2018
@petebacondarwin petebacondarwin moved this from REVIEW to MERGE in docs-infra Mar 14, 2018
@kara
Copy link
Contributor

kara commented Mar 14, 2018

@petebacondarwin Looks like this has a lint error?

@kara kara removed the action: merge The PR is ready for merge by the caretaker label Mar 14, 2018
@petebacondarwin
Copy link
Member Author

Doh!

@petebacondarwin
build(aio): improve the API Pipe pages
4d5b65b 
This change adds:

* an impure badge for Pipes that are marked as  `pure: false`
* a pipe specific overview that shows the syntax for using a pipe in a template.
* an "input value" section describing the type of the value that the pipe expects.
* a "pipe params" section describing any additional params that a pipe expects.
@petebacondarwin
fixup! build(aio): improve the API Pipe pages
9e2ba6c
@petebacondarwin
fixup! build(aio): improve the API Pipe pages
f5448bf
@mary-poppins
Copy link

You can preview f5448bf at https://pr22702-f5448bf.ngbuilds.io/.

@kara kara added the action: merge The PR is ready for merge by the caretaker label Mar 14, 2018
@kara kara closed this in d509bd6 Mar 14, 2018
@petebacondarwin
Copy link
Member Author

Oh wow! Thanks @kara - you spotted I fixed it. That is super efficient.

@petebacondarwin petebacondarwin deleted the aio-api-pipes branch March 14, 2018 21:51
@petebacondarwin petebacondarwin removed this from MERGE in docs-infra Mar 15, 2018
leo6104 pushed a commit to leo6104/angular that referenced this pull request Mar 25, 2018
@petebacondarwin @leo6104
build(aio): improve the API Pipe pages (angular#22702)
cda9a79 
This change adds:

* an impure badge for Pipes that are marked as  `pure: false`
* a pipe specific overview that shows the syntax for using a pipe in a template.
* an "input value" section describing the type of the value that the pipe expects.
* a "pipe params" section describing any additional params that a pipe expects.

PR Close angular#22702
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Sep 13, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@gkalpak gkalpak gkalpak approved these changes

Assignees

@petebacondarwin petebacondarwin

Labels
action: merge The PR is ready for merge by the caretaker cla: yes target: major This PR is targeted for the next major release
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
5 participants
@petebacondarwin @mary-poppins @kara @gkalpak @googlebot