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

API template - overloads api page cleanup #24976

Closed
wants to merge 7 commits into from

Conversation

sjtrimble
Copy link
Contributor

@sjtrimble sjtrimble commented Jul 19, 2018

  • API page clean up primarily focusing on new overloads design
  • Moved some SCSS files to corresponding sections (some were in the modules section and should be in the page-specific sections)

Functionality Todos:

  • Add relevant index of overload
  • Make individual overload collapsable
  • Show only the first overload expanded, rest collapsed
  • Text should change to 'collapse all' once 'show all' is selected
  • Need to fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed
  • Need to display object ... as the and type , and the first sentence of the description should be "This is an anonymous type with a signature (link) described above.".

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: #24349

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

@sjtrimble sjtrimble added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 19, 2018
@sjtrimble sjtrimble self-assigned this Jul 19, 2018
@mary-poppins
Copy link

You can preview 2bc5c65 at https://pr24976-2bc5c65.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 8dfcc78 at https://pr24976-8dfcc78.ngbuilds.io/.

@petebacondarwin petebacondarwin added this to REVIEW in docs-infra Jul 21, 2018
@petebacondarwin
Copy link
Member

I'm afraid this needs a significant rebase.
Will the additional todos be completed in another PR?
Visually the "overloads" H4 looks smaller and less significant than the H5 headings of each overload, which seems wrong.
What else has changed, it is not so easy to tell from the diff. Perhaps some before and after images of significant changes would be helpful to speed up the approval process?

@petebacondarwin petebacondarwin added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews comp: docs-infra and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 23, 2018
@petebacondarwin petebacondarwin added this to the Backlog milestone Jul 23, 2018
@ngbot ngbot bot modified the milestones: Backlog, needsTriage Jul 23, 2018
@sjtrimble
Copy link
Contributor Author

@petebacondarwin
Here are some of the changes visualized (before/after):

Methods Section Changes
methods-comparison

  • Edit/source code icons blue to match link styling
  • Smaller code font to match rest of text size
  • Removal of box/border around parameters table

Overloads Section Changes
overload-initial

  • Edit/source code icons blue to match link styling
  • Larger heading size for Overloads section heading
  • Removal of box/border around overloads section heading
  • Expand/collapse caret icon accompanied by text for more clear instructions (as individual overload item section will have their own collapse/expand sections in the new design)

Overload Details Changes
overload-detail

  • Added overload item heading with # of # to easily better distinguish between individual overload sections
  • Smaller code font to match rest of text size
  • Removal of box/border around parameters table
  • Updated styling of individual overload section sub-headings so content hierarchy is a bit more clear

@sjtrimble
Copy link
Contributor Author

@petebacondarwin

  1. Regarding the additional to-do's, I was hoping you or someone on the team could help :)
  2. Overloads heading is an error and I will fix
  3. I will try to rebase and see how it goes.

@sjtrimble
Copy link
Contributor Author

Design for reference:
api-page-overloads

@sjtrimble
Copy link
Contributor Author

Overloads parameter table design discussed Jul 26 (for reference):

overloads-param-table-design

@IgorMinar
Copy link
Contributor

IgorMinar commented Jul 26, 2018

the second option - indent & no color - looks good to me.

the option without indentation looks better on this mock, but once it's in the context of the entire page the lack of indentation makes the visual hierarchy harder to see, so that's why I think we should try to stick with the indentation for now.

@mary-poppins
Copy link

You can preview a8f5cd5 at https://pr24976-a8f5cd5.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 07bd5c7 at https://pr24976-07bd5c7.ngbuilds.io/.

@sjtrimble
Copy link
Contributor Author

@petebacondarwin

I rebased, and made the changes based on the mockup we agreed on.
There's a list of Functionality Todos in the description above that covers all of the items that are missing from this PR that I'll need help with :)

@petebacondarwin petebacondarwin self-assigned this Aug 9, 2018
@petebacondarwin
Copy link
Member

petebacondarwin commented Aug 13, 2018

  • Add relevant index of overload
  • Make individual overload collapsable
  • Show only the first overload expanded, rest collapsed
  • Text should change to 'collapse all' once 'show all' is selected
  • Need to fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed
  • Need to display object ... as the and type , and the first sentence of the description should be "This is an anonymous type with a signature (link) described above.".

For this last item the docs for the HttpClient will need to be altered to add in docs for the option parameter. E.g.

  /**
   * Construct a request which interprets the body as an `ArrayBuffer` and returns it.
   *
   * @param options This is an anonymous type with a signature described in the
   * [usage notes](#request-usage-notes).
   * @return an `Observable` of the body as an `ArrayBuffer`.
   */
  request(method: string, url: string, options: {
    body?: any,
    headers?: HttpHeaders|{[header: string]: string | string[]},
    observe?: 'body',
    params?: HttpParams|{[param: string]: string | string[]},
    reportProgress?: boolean,
    responseType: 'arraybuffer', withCredentials?: boolean,
  }): Observable<ArrayBuffer>;

@mary-poppins
Copy link

You can preview da6d2bf at https://pr24976-da6d2bf.ngbuilds.io/.

@petebacondarwin
Copy link
Member

Another review needed please @gkalpak (according to PullApprove).

@kara kara added cla: yes and removed cla: no labels Nov 1, 2018
@googlebot
Copy link

A Googler has manually verified that the CLAs look good.

(Googler, please make sure the reason for overriding the CLA status is clearly documented in these comments.)

kara pushed a commit that referenced this pull request Nov 1, 2018
kara pushed a commit that referenced this pull request Nov 1, 2018
kara pushed a commit that referenced this pull request Nov 1, 2018
* Make individual overloads collapsible
* Show only the first overload expanded, rest collapsed
* Text changes to 'collapse all' once 'show all' is clicked
* Fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed

PR Close #24976
kara pushed a commit that referenced this pull request Nov 1, 2018
…24976)

In some overloads, the parameter type can be a large anonymous
object type.
This change displays such types as `object`. It is then up to the
documentation author to put more information about the type in the
method usage notes.

PR Close #24976
kara pushed a commit that referenced this pull request Nov 1, 2018
@kara kara closed this in 6902977 Nov 1, 2018
kara pushed a commit that referenced this pull request Nov 1, 2018
kara pushed a commit that referenced this pull request Nov 1, 2018
* Make individual overloads collapsible
* Show only the first overload expanded, rest collapsed
* Text changes to 'collapse all' once 'show all' is clicked
* Fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed

PR Close #24976
kara pushed a commit that referenced this pull request Nov 1, 2018
…24976)

In some overloads, the parameter type can be a large anonymous
object type.
This change displays such types as `object`. It is then up to the
documentation author to put more information about the type in the
method usage notes.

PR Close #24976
kara pushed a commit that referenced this pull request Nov 1, 2018
sculove pushed a commit to sculove/angular that referenced this pull request Nov 2, 2018
sculove pushed a commit to sculove/angular that referenced this pull request Nov 2, 2018
sculove pushed a commit to sculove/angular that referenced this pull request Nov 2, 2018
* Make individual overloads collapsible
* Show only the first overload expanded, rest collapsed
* Text changes to 'collapse all' once 'show all' is clicked
* Fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed

PR Close angular#24976
sculove pushed a commit to sculove/angular that referenced this pull request Nov 2, 2018
…ngular#24976)

In some overloads, the parameter type can be a large anonymous
object type.
This change displays such types as `object`. It is then up to the
documentation author to put more information about the type in the
method usage notes.

PR Close angular#24976
sculove pushed a commit to sculove/angular that referenced this pull request Nov 2, 2018
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
* Make individual overloads collapsible
* Show only the first overload expanded, rest collapsed
* Text changes to 'collapse all' once 'show all' is clicked
* Fix chevron/carrot rotation animation when overloads / overload item is expanded or collapsed

PR Close angular#24976
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
…ngular#24976)

In some overloads, the parameter type can be a large anonymous
object type.
This change displays such types as `object`. It is then up to the
documentation author to put more information about the type in the
method usage notes.

PR Close angular#24976
FrederikSchlemmer pushed a commit to FrederikSchlemmer/angular that referenced this pull request Jan 3, 2019
@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 14, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker cla: yes jira-sync target: patch This PR is targeted for the next patch release
Projects
Development

Successfully merging this pull request may close these issues.

None yet

8 participants