feat(docs-infra): add getting started widgets by brandonroberts · Pull Request #26059 · angular/angular

brandonroberts · 2018-09-21T16:04:29Z

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
[ ] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[x] angular.io application / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

Small interactive widget components for getting started

mary-poppins · 2018-09-21T16:09:19Z

You can preview 2b8eaa9 at https://pr26059-2b8eaa9.ngbuilds.io/.

gkalpak

The tests seem a little off. I didn't look through all of them carefully. The rest looks reasonable, but it would be better if we could see them in action 😉

Some observations:

Too much observables. Often seem unnecessary.
It would be nice if we could get rid of .template and derive it from the .result HTML. (That would make them more future proof too.)
If we expect to have more of these components in the future, it might make sense to make them more generic/configuration based. But they are fine for now imo.

gkalpak · 2018-09-21T16:28:56Z

mary-poppins · 2018-09-24T15:32:18Z

You can preview 9f76b82 at https://pr26059-9f76b82.ngbuilds.io/.

mary-poppins · 2018-09-24T16:11:33Z

You can preview a4abdcf at https://pr26059-a4abdcf.ngbuilds.io/.

mary-poppins · 2018-09-24T16:50:58Z

You can preview fb59be2 at https://pr26059-fb59be2.ngbuilds.io/.

mary-poppins · 2018-09-24T17:28:13Z

You can preview 9f50303 at https://pr26059-9f50303.ngbuilds.io/.

brandonroberts · 2018-09-24T18:26:40Z

@gkalpak I've address your feedback with the observables and unnecessary tests. I didn't do anything with deriving the template from the results. I'll have to get some guidance add follow that with another PR. I'd like to get this landed first for Stephen, so we can iterate on the other getting started guide.

brandonroberts · 2018-09-24T18:27:21Z

@gkalpak You can see the widgets on the /tutorial page temporarily. I will remove them before the PR lands.

mary-poppins · 2018-09-24T18:28:16Z

You can preview 34bddba at https://pr26059-34bddba.ngbuilds.io/.

mary-poppins · 2018-09-24T18:30:50Z

You can preview 5db11b0 at https://pr26059-5db11b0.ngbuilds.io/.

gkalpak

BTW, I would consider changing all widget selectors to ...-gs-... (instead of ...-getting-started-... for consistency. (The directory name should be a strong enough hint as to what gs stands for.)

gkalpak · 2018-09-25T09:09:31Z

What is this for?

Its needed for the tooltip that displayed when the JSON in invalid. I believe Angular Material needs to be at least version 6.1.x to lazy load the OverlayModule.

So, importing MatTooltipModule is not enough? That's weird. Is it because we lazy-load MatTooltipModule?

Its not enough to import MatTooltipModule. It throws an error with the Overlay provider being missing.

Misunderstood. Yes its because MatTooltipModule is lazy loaded.

gkalpak · 2018-09-25T09:36:35Z

BTW, you can add the widgets in test.html (for showcase/review purposes).

mary-poppins · 2018-09-25T16:58:40Z

You can preview 536d2fa at https://pr26059-536d2fa.ngbuilds.io/.

mary-poppins · 2018-09-25T19:37:37Z

You can preview 29fcf4c at https://pr26059-29fcf4c.ngbuilds.io/.

mary-poppins · 2018-10-24T19:42:34Z

You can preview f25f968 at https://pr26059-f25f968.ngbuilds.io/.

mary-poppins · 2018-10-25T12:17:18Z

You can preview de01285 at https://pr26059-de01285.ngbuilds.io/.

IgorMinar

looks good now on mobile. thanks!

I'm not happy about the size in increase, but we'll investigate that more once we switch to ivy.

Can you please:

rebase on master
squash into a single commit
prefix the commit with feat(docs-infra): ("docs(docs-infra)" means documentation for docs-infrastruture).

thanks!

mary-poppins · 2018-11-02T14:08:15Z

You can preview 4f2014d at https://pr26059-4f2014d.ngbuilds.io/.

PR Close #26059

angular-automatic-lock-bot · 2019-09-14T06:52:32Z

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

brandonroberts requested review from gkalpak and petebacondarwin September 21, 2018 16:04

googlebot added the cla: yes label Sep 21, 2018

gkalpak reviewed Sep 21, 2018

View reviewed changes

kara added the state: WIP label Sep 22, 2018

brandonroberts force-pushed the getting-started-widgets branch from 2b8eaa9 to 9f76b82 Compare September 24, 2018 15:24

brandonroberts force-pushed the getting-started-widgets branch from a4abdcf to fb59be2 Compare September 24, 2018 16:42

brandonroberts force-pushed the getting-started-widgets branch from 34bddba to 5db11b0 Compare September 24, 2018 18:23

brandonroberts changed the title ~~WIP: docs(docs-infra): add getting started widgets~~ docs(docs-infra): add getting started widgets Sep 24, 2018

jenniferfell added this to the v7.0 milestone Sep 24, 2018

ngbot Bot removed this from the v7.0 milestone Sep 24, 2018

gkalpak suggested changes Sep 25, 2018

View reviewed changes

brandonroberts force-pushed the getting-started-widgets branch from 536d2fa to 29fcf4c Compare September 25, 2018 19:27

brandonroberts added comp: docs target: major This PR is targeted for the next major release labels Sep 25, 2018

brandonroberts requested review from StephenFluin and removed request for petebacondarwin September 26, 2018 12:54

IgorMinar self-assigned this Oct 24, 2018

brandonroberts force-pushed the getting-started-widgets branch from d98e78e to f25f968 Compare October 24, 2018 19:37

brandonroberts added target: patch This PR is targeted for the next patch release and removed target: major This PR is targeted for the next major release labels Oct 24, 2018

ngbot Bot added this to the needsTriage milestone Oct 24, 2018

brandonroberts force-pushed the getting-started-widgets branch from f25f968 to de01285 Compare October 25, 2018 12:13

IgorMinar 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 Nov 2, 2018

IgorMinar approved these changes Nov 2, 2018

View reviewed changes

brandonroberts force-pushed the getting-started-widgets branch from de01285 to 62244a8 Compare November 2, 2018 13:48

brandonroberts removed the request for review from StephenFluin November 2, 2018 13:48

brandonroberts changed the title ~~docs(docs-infra): add getting started widgets~~ feat(docs-infra): add getting started widgets Nov 2, 2018

feat(docs-infra): add getting started widgets

4f2014d

brandonroberts force-pushed the getting-started-widgets branch from 62244a8 to 4f2014d Compare November 2, 2018 14:04

brandonroberts added feature Label used to distinguish feature request from other issues effort3: weeks risk: low 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 Nov 2, 2018

ngbot Bot modified the milestones: needsTriage, Backlog Nov 2, 2018

brandonroberts added the action: merge The PR is ready for merge by the caretaker label Nov 2, 2018

kara pushed a commit that referenced this pull request Nov 2, 2018

feat(docs-infra): add getting started widgets (#26059)

fe1c0d7

PR Close #26059

kara closed this in affcbbd Nov 2, 2018

brandonroberts deleted the getting-started-widgets branch November 2, 2018 18:11

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

Conversation

brandonroberts commented Sep 21, 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?

Other information

Uh oh!

mary-poppins commented Sep 21, 2018

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gkalpak Sep 21, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

brandonroberts commented Sep 24, 2018

Uh oh!

brandonroberts commented Sep 24, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

mary-poppins commented Sep 24, 2018

Uh oh!

gkalpak left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

gkalpak Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

brandonroberts Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

gkalpak Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

brandonroberts Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

brandonroberts Sep 25, 2018

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gkalpak commented Sep 25, 2018

Uh oh!

mary-poppins commented Sep 25, 2018

Uh oh!

mary-poppins commented Sep 25, 2018

Uh oh!

mary-poppins commented Oct 24, 2018

Uh oh!

mary-poppins commented Oct 25, 2018

Uh oh!

IgorMinar left a comment

brandonroberts commented Sep 21, 2018 •

edited

Loading