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

fix(docs-infra): make API member name bold #31574

Closed
wants to merge 4 commits into from

Conversation

sjtrimble
Copy link
Contributor

@sjtrimble sjtrimble commented Jul 15, 2019

  • Remove linenums on API interface overview code block Addressed in a different PR.
  • Return API member name bold font weight
  • SCSS formatting cleanup

Closes #31494

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • 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?

#31494

Issue Number: #31494

Does this PR introduce a breaking change?

  • Yes
  • No

@sjtrimble sjtrimble added action: review The PR is still awaiting reviews from at least one requested reviewer comp: docs-infra labels Jul 15, 2019
@sjtrimble sjtrimble requested a review from a team as a code owner July 15, 2019 21:13
@sjtrimble sjtrimble self-assigned this Jul 15, 2019
@ngbot ngbot bot added this to the needsTriage milestone Jul 15, 2019
@mary-poppins
Copy link

You can preview c938e43 at https://pr31574-c938e43.ngbuilds.io/.

@mary-poppins
Copy link

You can preview 99df6be at https://pr31574-99df6be.ngbuilds.io/.

@gkalpak gkalpak added state: WIP target: patch This PR is targeted for the next patch release and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Jul 16, 2019
aio/src/styles/2-modules/_api-list.scss Outdated Show resolved Hide resolved
aio/src/styles/2-modules/_api-pages.scss Outdated Show resolved Hide resolved
aio/src/styles/2-modules/_api-pages.scss Show resolved Hide resolved
aio/src/styles/2-modules/_api-pages.scss Outdated Show resolved Hide resolved
@sjtrimble sjtrimble added the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 19, 2019
@mary-poppins
Copy link

You can preview 7ad9a62 at https://pr31574-7ad9a62.ngbuilds.io/.

@gkalpak gkalpak removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Jul 20, 2019
@sjtrimble sjtrimble added the action: review The PR is still awaiting reviews from at least one requested reviewer label Aug 2, 2019
@mary-poppins
Copy link

You can preview 169a80d at https://pr31574-169a80d.ngbuilds.io/.

@gkalpak gkalpak removed the action: review The PR is still awaiting reviews from at least one requested reviewer label Aug 3, 2019
@gkalpak gkalpak added this to IN PROGRESS in docs-infra Aug 3, 2019
Copy link
Member

@gkalpak gkalpak left a comment

Choose a reason for hiding this comment

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

One minor nit, plus that the last fixup commit (d6b76d1) does not match the original commit. It's header is fixup! fix(docs-infra): return deprecated styles for api list instead of fixup! fix(docs-infra): remove lineums on api interface overview.

@sjtrimble, once you've made the fixes, feel free to remove the cleanup label and add the merge label 😉

@@ -205,7 +205,7 @@ aio-api-list {
color: $blue-500;
}

&.deprecated-api-item {
&.deprecated-api-item {
Copy link
Member

Choose a reason for hiding this comment

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

Incorrect indentation 😁

@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 state: WIP labels Aug 6, 2019
@gkalpak gkalpak removed the request for review from petebacondarwin August 6, 2019 15:16
@mary-poppins
Copy link

You can preview dd5e430 at https://pr31574-dd5e430.ngbuilds.io/.

sjtrimble and others added 2 commits March 6, 2020 14:39
- Return API member name bold font weight
- SCSS formatting cleanup

Closes angular#31494
@gkalpak gkalpak changed the title fix(docs-infra): remove linenums on API interface overview fix(docs-infra): make API member name bold Mar 6, 2020
@mary-poppins
Copy link

You can preview 0491bf1 at https://pr31574-0491bf1.ngbuilds.io/.

This commit merges the two `.api-body` style blocks, removing duplicate
styles and re-ordering sub-blocks (to group relevant styles together).
It should not affect styling.
@gkalpak
Copy link
Member

gkalpak commented Mar 6, 2020

@googlebot I consent.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes and removed cla: no labels Mar 6, 2020
@@ -294,18 +294,6 @@
}
}

.github-links {
Copy link
Member

Choose a reason for hiding this comment

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

There is a practically identical block further up this file.

@gkalpak gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer type: bug/fix 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 6, 2020
@gkalpak
Copy link
Member

gkalpak commented Mar 6, 2020

Thx for the ping, @ajitsinghkaler 👍

I went ahead and cleaned it up myself, because you wouldn't have enough permissions to update the PR. While doing so, I realized that the linenums for interface overview had already been removed in an earlier PR, so this PR was mainly making API member name bold (and cleaning up SCSS code). I updated the commit messages to reflect that.

I also noticed that the _api-pages.scss code needed further clean-up (remove duplicate style and merge style-blocks). So, I pushed another two commits to address that. (My two commits should not have any impact on styling - just refactoring.)

@sjtrimble, @ajitsinghkaler: Please take a look at the changes/preview to make sure I didn't accidentally mess something up 🙏

@mary-poppins
Copy link

You can preview af4aec9 at https://pr31574-af4aec9.ngbuilds.io/.

Copy link
Contributor

@ajitsinghkaler ajitsinghkaler left a comment

Choose a reason for hiding this comment

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

Type-alias api type highlighting is still left

@gkalpak most of it looks good just one question we have highlighted interface's classes elements
Desktop screenshot (2)

When I was checking out I saw type alias elements are not being highlighted any specific reason for that I think its best we highlight them too for consistency I 've attached screenshots for your reference.

Desktop screenshot (1)

Rest LGTM.

@gkalpak gkalpak added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Mar 11, 2020
@gkalpak
Copy link
Member

gkalpak commented Mar 11, 2020

Thx for taking a look, @ajitsinghkaler! I don't think we can fix this, because there doesn't seem to be a way to target type-alias members atm. For interfaces, they are identified as members and we can attach the member-name class to them, but type-aliases we don't have such granular tokenization.

I am not sure whether this is a limitation of TypeScript or whether it is something that can be fixed in dgeni, but in any case I believe this is outside of the scope of this PR.

Based on your feedback, I am marking this for merging and we can address any additional concerns/improvements in follow-up PRs.

@matsko matsko closed this in cffbaba Mar 11, 2020
matsko pushed a commit that referenced this pull request Mar 11, 2020
This commit merges the two `.api-body` style blocks, removing duplicate
styles and re-ordering sub-blocks (to group relevant styles together).
It should not affect styling.

PR Close #31574
matsko pushed a commit that referenced this pull request Mar 11, 2020
- Return API member name bold font weight
- SCSS formatting cleanup

Closes #31494

PR Close #31574
matsko pushed a commit that referenced this pull request Mar 11, 2020
This commit merges the two `.api-body` style blocks, removing duplicate
styles and re-ordering sub-blocks (to group relevant styles together).
It should not affect styling.

PR Close #31574
@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 Apr 11, 2020
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 target: patch This PR is targeted for the next patch release type: bug/fix
Projects
docs-infra
IN PROGRESS
Development

Successfully merging this pull request may close these issues.

docs-infra: class/interface overview styling has changed
5 participants