feat(aio): add "suggest edits" feature to all docs by martinsik · Pull Request #24378 · angular/angular

martinsik · 2018-06-08T16:15:25Z

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/master/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

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?

The "Suggest Edits" pencil icon is only in the API subsection.

Issue Number: #23981

What is the new behavior?

The "Suggest Edits" pencil icon is on all pages under /docs.

Does this PR introduce a breaking change?

[ ] Yes
[X] No

mary-poppins · 2018-06-08T16:44:55Z

You can preview a8b1b67 at https://pr24378-a8b1b67.ngbuilds.io/.

gkalpak

Please change the scope in commit messages to docs-infra.
Other than that, it looks good from a quick look 💯

gkalpak · 2018-06-08T16:43:30Z

aio is not an appropriate scope (it is only for documentation of the docs-viewer app itself or the documentation tooling; and it has been renamed to docs-infra to make it clear).
For guides and tutorial, it should be just docs: ...

gkalpak · 2018-06-08T16:50:13Z

One more thing: I would like the button style to be closer to #24000 😁

petebacondarwin · 2018-06-08T21:09:01Z

One more, more thing: I would rather keep helper template files (i.e. ones that are not used directly as templates) such as the githubLinks.html file out of the main folder. I understand you moved it because it is shared outside the API stuff now. Perhaps you could create a lib folder directly below the templates folder for this file?

martinsik · 2018-06-09T13:47:16Z

@gkalpak I updated commit messages. The aio scope is actually mentioned here https://github.com/angular/angular/blob/master/CONTRIBUTING.md#scope. Now the links use just docs: prefix. My changes will probably conflict with #24000 but I think visually the buttons look the same if that's what you meant.

@petebacondarwin I moved the githubLinks.html file to a new lib dir.

I also added e2e tests for the "Suggest edit" buttons under /docs.

There was one very confusing thing. The doc.fileInfo template variable has different structure for doc.docType === 'content' and for everything else.

With content its structure comes from here https://github.com/angular/dgeni-packages/blob/8c856f3d75bb527248f86e6c70d3c6ed81e7740c/base/processors/read-files.js#L70 while all the other page types use this https://github.com/angular/dgeni-packages/blob/c7d1b54c808931394912fcf062df25aeeb572883/typescript/src/services/TsParser/FileInfo.ts. The first one has projectRelativePath while the second one realProjectRelativePath so that's why I had to add an extra condition for this.

mary-poppins · 2018-06-09T13:58:39Z

You can preview 727934c at https://pr24378-727934c.ngbuilds.io/.

mary-poppins · 2018-06-10T08:04:38Z

You can preview 3d093b2 at https://pr24378-3d093b2.ngbuilds.io/.

lacolaco · 2018-06-11T02:39:39Z

If this feature will be released, some angular.io localization sites (at least our angular.jp) will customize it to jump to its own repository.
So I want this specs not to depend on angular/angular repository.

@lacolaco The same regexps are already used to test API's buttons:
https://github.com/angular/angular/blob/master/aio/tests/e2e/api.e2e-spec.ts#L60-L68

Exactly. After I've send the above comment, I found it's e2e testing so it doesn't affect to build aio. Sorry, and thank you. 🙌

gkalpak · 2018-06-11T09:28:00Z

The aio scope is actually mentioned here https://github.com/angular/angular/blob/master/CONTRIBUTING.md#scope.

Ooops, we missed it in #24295. It should be docs-infra. (Submitted #24410.)

My changes will probably conflict with #24000 but I think visually the buttons look the same if that's what you meant.

AFAICT, the buttons here are blue (gray on hover), while the buttons on #24000 are gray (blue on hover).

martinsik · 2018-06-11T12:22:56Z

@gkalpak When I check the latest #24000 build at https://pr24000-9697d53.ngbuilds.io/ I see blue buttons (and grey on hover).

gkalpak · 2018-06-11T12:30:42Z

Nevermind. I was referring to the buttons for methods. I didn't realize/remember we have differently styled buttons for the whole doc. This is a different discussion, so the buttons are fine as you have them (for this PR's purpose). 👍

petebacondarwin

LGTM - please rebase and fix the conflicts.

martinsik · 2018-06-19T19:05:31Z

@petebacondarwin @gkalpak rebased and conflicts fixed.

mary-poppins · 2018-06-19T19:27:57Z

You can preview 2ae87a0 at https://pr24378-2ae87a0.ngbuilds.io/.

petebacondarwin · 2018-07-22T06:43:35Z

What is the motivation for this change? Other than the marketing docs, which docs should not have this link?

@petebacondarwin It's been a while since I did this but I think this is because the edit icon should be visible almost everywhere including pages in the top level navigation such as /docs but not at /features or /resources (I think all these pages use the same content.template.html)

petebacondarwin · 2018-08-29T16:02:42Z

Caretaker: it appears that the rebase caused this PR's CLA status to go red. It looks to me as though it should be green. Also the last pending Travis task is flaky and nothing to do with this PR.

ngbot · 2018-08-29T16:03:46Z

I see that you just added the PR action: merge label, but the following checks are still failing:
     missing required label: "cla: yes"
     forbidden label detected: cla: no
     status "continuous-integration/travis-ci/pr" is pending

If you want your PR to be merged, it has to pass all the CI checks.

If you can't get the PR to a green state due to flakes or broken master, please try rebasing to master and/or restarting the CI job. If that fails and you believe that the issue is not due to your change, please contact the caretaker and ask for help.

googlebot · 2018-08-31T06:57:08Z

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.)

…irs (#24378) PR Close #24378

PR Close #24378

…ere expected (#24378) PR Close #24378

PR Close #24378

…irs (#24378) PR Close #24378

PR Close #24378

…ere expected (#24378) PR Close #24378

gkalpak · 2018-09-05T08:26:13Z

🎉

angular-automatic-lock-bot · 2019-09-14T00:51:30Z

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.}

googlebot added the cla: yes label Jun 8, 2018

gkalpak added action: review The PR is still awaiting reviews from at least one requested reviewer comp: docs-infra target: patch This PR is targeted for the next patch release labels Jun 8, 2018

gkalpak requested a review from petebacondarwin June 8, 2018 16:44

gkalpak reviewed Jun 8, 2018

View reviewed changes

martinsik force-pushed the issue-23981 branch from a8b1b67 to 727934c Compare June 9, 2018 13:49

martinsik force-pushed the issue-23981 branch from 727934c to 3d093b2 Compare June 10, 2018 07:51

lacolaco reviewed Jun 11, 2018

View reviewed changes

gkalpak approved these changes Jun 11, 2018

View reviewed changes

petebacondarwin 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 Jun 15, 2018

petebacondarwin suggested changes Jun 15, 2018

View reviewed changes

petebacondarwin added this to the Backlog milestone Jun 15, 2018

ngbot Bot modified the milestones: Backlog, needsTriage Jun 15, 2018

martinsik force-pushed the issue-23981 branch from 3d093b2 to 2ae87a0 Compare June 19, 2018 19:02

petebacondarwin reviewed Jul 22, 2018

View reviewed changes

petebacondarwin added merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note 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 Aug 29, 2018

petebacondarwin added the action: merge The PR is ready for merge by the caretaker label Aug 29, 2018

sjtrimble added subtype: docs-design and removed subtype: docs-design labels Aug 31, 2018

mhevery added cla: yes and removed cla: no labels Aug 31, 2018

mhevery closed this in 9533cc9 Aug 31, 2018

mhevery pushed a commit that referenced this pull request Aug 31, 2018

fix(docs-infra): show "suggest edits" only for /guide and /tutorial d…

b89a7dd

…irs (#24378) PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

refactor(docs-infra): refactor templates (#24378)

d539122

PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

test(docs-infra): test that the "suggest edit" buttons are visible wh…

ccb4a39

…ere expected (#24378) PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

feat(docs-infra): add "suggest edits" feature to all docs (#24378)

82088a8

PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

fix(docs-infra): show "suggest edits" only for /guide and /tutorial d…

66b7870

…irs (#24378) PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

refactor(docs-infra): refactor templates (#24378)

ff78149

PR Close #24378

mhevery pushed a commit that referenced this pull request Aug 31, 2018

test(docs-infra): test that the "suggest edit" buttons are visible wh…

98f5ace

…ere expected (#24378) PR Close #24378

petebacondarwin mentioned this pull request Aug 31, 2018

Add "suggest edits" feature to all docs #23981

Closed

angular-automatic-lock-bot Bot locked and limited conversation to collaborators Sep 14, 2019

Conversation

martinsik commented Jun 8, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Uh oh!

mary-poppins commented Jun 8, 2018

Uh oh!

gkalpak left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

gkalpak Jun 8, 2018

Choose a reason for hiding this comment

Uh oh!

gkalpak commented Jun 8, 2018

Uh oh!

petebacondarwin commented Jun 8, 2018

Uh oh!

martinsik commented Jun 9, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Jun 9, 2018

Uh oh!

mary-poppins commented Jun 10, 2018

Uh oh!

lacolaco Jun 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

martinsik Jun 11, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lacolaco Jun 11, 2018

Choose a reason for hiding this comment

Uh oh!

gkalpak commented Jun 11, 2018

Uh oh!

martinsik commented Jun 11, 2018

Uh oh!

gkalpak commented Jun 11, 2018

Uh oh!

petebacondarwin left a comment

Choose a reason for hiding this comment

Uh oh!

martinsik commented Jun 19, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mary-poppins commented Jun 19, 2018

Uh oh!

petebacondarwin Jul 22, 2018

Choose a reason for hiding this comment

Uh oh!

martinsik Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

petebacondarwin commented Aug 29, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ngbot Bot commented Aug 29, 2018

Uh oh!

googlebot commented Aug 31, 2018

Uh oh!

gkalpak commented Sep 5, 2018

Uh oh!

angular-automatic-lock-bot Bot commented Sep 14, 2019

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

martinsik commented Jun 8, 2018 •

edited

Loading

gkalpak left a comment •

edited

Loading

martinsik commented Jun 9, 2018 •

edited

Loading

lacolaco Jun 11, 2018 •

edited

Loading

martinsik Jun 11, 2018 •

edited

Loading

martinsik commented Jun 19, 2018 •

edited

Loading

martinsik Aug 29, 2018 •

edited

Loading

petebacondarwin commented Aug 29, 2018 •

edited

Loading