docs(toh-6) move styles from sample.css to heroes.component.css #1751

Adam-Meisen · 2016-06-26T12:30:46Z

As outlined in #1750 and touched upon in #1686, sample.css is used in the examples but never actually mentioned in the tutorial. This moves the contents of sample.css to heroes.component.css and updates the tutorial to reflect this change. I've attempted to follow the style of the rest of the page as closely as possible.

contents of sample.css moved to heroes.component.css
appropriate #docregions added to heroes.component.css to render the file properly in makeExample and makeTabs mixins
Tutorial edited to instruct the reader to add the rules formerly contained in sample.css to heroes.component.css
heroes.component.css has been added to the summary of changed files
sample.css has been removed from the list of files at the bottom of the page
sample.css has been deleted as it is no longer needed

googlebot · 2016-06-26T12:30:47Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

googlebot · 2016-06-26T12:30:47Z

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll verify. Thanks.

If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
If you signed the CLA as a corporation, please let us know the company's name.

Adam-Meisen · 2016-06-26T12:35:02Z

I signed it!
On Jun 26, 2016 7:30 AM, "googlebot" [email protected] wrote:

Thanks for your pull request. It looks like this may be your first
contribution to a Google open source project. Before we can look at your
pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/
https://cla.developers.google.com/ to sign.

Once you've signed, please reply here (e.g. I signed it!) and we'll

verify. Thanks.

If you've already signed a CLA, it's possible we don't have your
GitHub username or you're using a different email address. Check your
existing CLA data https://cla.developers.google.com/clas and verify
that your email is set on your git commits
https://help.github.com/articles/setting-your-email-in-git/.

If you signed the CLA as a corporation, please let us know the
company's name.

—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
#1751 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AA-2UCbWlTKKcneF8bNVCY_jmk_ineZxks5qPnD8gaJpZM4I-i5r
.

googlebot · 2016-06-26T12:35:04Z

CLAs look good, thanks!

googlebot · 2016-06-26T12:35:04Z

CLAs look good, thanks!

googlebot · 2016-06-28T00:59:35Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-06-28T00:59:35Z

We found a Contributor License Agreement for you (the sender of this pull request) and all commit authors, but as best as we can tell these commits were authored by someone else. If that's the case, please add them to this pull request and have them confirm that they're okay with these commits being contributed to Google. If we're mistaken and you did author these commits, just reply here to confirm.

googlebot · 2016-06-28T01:14:19Z

CLAs look good, thanks!

googlebot · 2016-06-28T01:14:19Z

CLAs look good, thanks!

Adam-Meisen · 2016-06-28T01:45:07Z

Rebased. Forgot to use -f the first time, angered googlebot.

Adam-Meisen · 2016-06-28T02:21:35Z

Squashed my commits, cleaned up the messages. Also accidentally closed the PR and opened it again. Woops.

Adam-Meisen · 2016-06-28T11:05:33Z

Something about the e2e tests for part 6 is bizarre, I've had two consecutive build failures with different errors each time. None of my changes should cause either of these.

[02:35:14] failed - should be able to save details from "Dashboard" view

[02:35:14] Failed: No element found using locator: By(css selector, input)

[08:31:57] failed - should be able to add a hero from the "Heroes" view

[08:31:57] Expected 0 to be 10, 'should show 10'.

Foxandxss · 2016-06-28T11:08:25Z

Yes, we fixed them in other PR

chalin · 2016-06-29T13:13:44Z

public/docs/_examples/toh-6/ts/app/heroes.component.css

@@ -57,3 +58,11 @@ button {
 button:hover {
  background-color: #cfd8dc;
 }
+/* #docregion delete-hero */


Rename the region to "additions" (since you'll be re-introducing the .error style that was inadvertently dropped --- see my comment on the original sample.css), and make the corresponding update in the .jade.

chalin · 2016-06-29T13:19:08Z

I have made some specific line comments. Please address those.

As I mentioned elsewhere, I like your proposal, but I'll need to discuss this with some of the doc team elders to find out the history of the sample.css files used in the various chapters.

cc @kwalrath

Adam-Meisen · 2016-06-29T21:24:44Z

No worries. 👍
Fixed and rebased. Thanks for the feedback.

chalin · 2016-08-01T19:42:15Z

@Adam-Meisen : oh, I thought that this had been merged already!

Could you rebase? Then I'll review and see if we can get this merged.

chalin · 2016-08-01T21:48:05Z

Hmm, just realized that this might need a little more work. Let me investigate and get back to you, @Adam-Meisen.

chalin · 2016-08-01T21:54:23Z

@Adam-Meisen Thanks for the rebase. Getting close! Notice that sample.css has two new styles, and they are now being lost. Could you add them to a (new) hero-search.component.css file, adjust the component, etc?

Adam-Meisen · 2016-08-01T22:43:48Z

Yep, working on it.

Adam-Meisen · 2016-08-02T06:01:27Z

public/docs/_examples/toh-6/ts/app/hero-search.component.css

@@ -1,11 +1,4 @@
 /* #docregion */


I don't know why git decided this was a rename operation when I rebased, but I don't think that hurts anything.

chalin · 2016-08-02T15:13:00Z

It's looking good other than the minor changes I pointed out in the line comments.

Adam-Meisen · 2016-08-02T16:49:29Z

public/docs/ts/latest/tutorial/toh-pt6.jade

@@ -396,7 +398,6 @@ block observables-section
    We start by creating `HeroSearchService` that sends search queries to our server's web api.

  +makeExample('toh-6/ts/app/hero-search.service.ts', null, 'app/hero-search.service.ts')(format=".")
-


This newline didn't seem to have any significance, but it messed with syntax highlighting.

chalin · 2016-08-02T17:06:27Z

Some edits just landed for toh-6. One more rebase to do.

chalin · 2016-08-02T17:09:57Z

public/docs/ts/latest/tutorial/toh-pt6.jade

@@ -316,7 +316,9 @@ block add-new-hero-via-detail-comp
  The user can *delete* an existing hero by clicking a delete button next to the hero's name.
  Add the following to the heroes component HTML right after the hero name in the repeated `<li>` tag:
 +makeExample('app/heroes.component.html', 'delete')
-
+:marked
+  Add the following CSS to the bottom of `heroes.component.css`.


Oh, I just noticed this. We need to make this sentence work for all language versions. I'd suggest going with:

Add the following to the bottom of the HeroesComponent CSS file:
+makeExcerpt('app/heroes.component.css', 'additions')

(Drop the last '' argument to makeExcerpt.)

Alrighty

On Tue, Aug 2, 2016, 12:10 PM Patrice Chalin [email protected]
wrote:

In public/docs/ts/latest/tutorial/toh-pt6.jade
#1751 (comment):

@@ -316,7 +316,9 @@ block add-new-hero-via-detail-comp
The user can delete an existing hero by clicking a delete button next to the hero's name.
Add the following to the heroes component HTML right after the hero name in the repeated <li> tag:

+makeExample('app/heroes.component.html', 'delete')

+:marked

Add the following CSS to the bottom of heroes.component.css.

Oh, I just noticed this. We need to make this sentence work for all
language versions. I'd suggest going with:

Add the following to the bottom of the HeroesComponent CSS file:
+makeExcerpt('app/heroes.component.css', 'additions')

(Drop the last '' argument to makeExcerpt.)

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/angular/angular.io/pull/1751/files/0320013bbf27da52ff1e3bd7bd7b7d155b697be2#r73196526,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AA-2UIHQBheeyaJ9PhZkxsF3UJxBto5Eks5qb3nqgaJpZM4I-i5r
.

Move additional styles from `sample.css` to `hero-search.component.css`. Edit tutorial to reflect the removal of `sample.css` and the change to `heroes.component.css`. Edit tutorial to reflect the addition of `hero-search.component.css` file. Edit `hero-search.component.ts` to include styles from `hero-search.component.css`. Remove reference to `/public/docs/_examples/toh-6/ts/sample.css` from `/public/docs/_examples/toh-6/ts/index.html`.

Adam-Meisen · 2016-08-02T19:53:32Z

Alright, hopefully I didn't break anything in this rebase.

Adam-Meisen · 2016-08-02T20:05:50Z

public/docs/ts/latest/tutorial/toh-pt6.jade

     toh-6/ts/app/hero-detail.component.ts,
     toh-6/ts/app/hero-detail.component.html,
     toh-6/ts/app/hero.service.ts,
-     toh-6/ts/app/in-memory-data.service.ts,
-     toh-6/ts/sample.css`,
+     toh-6/ts/app/in-memory-data.service.ts`,


Should the hero-search.component files be added here?

As a separate makeTabs is ok (since there can be no more than 8 entries).

chalin · 2016-08-02T23:17:57Z

@Foxandxss @wardbell : LGTM. Tests are passing. Can we please merge this before the RC5 wave?

wardbell · 2016-08-05T23:45:20Z

THANK YOU! A lot of hard work went into getting this right. Much appreciated and happy to merge.

googlebot added the CLA: no label Jun 26, 2016

googlebot added CLA: yes and removed CLA: no labels Jun 26, 2016

googlebot added CLA: no and removed CLA: yes labels Jun 28, 2016

Adam-Meisen force-pushed the master branch from 3bcaf29 to 2df1914 Compare June 28, 2016 01:14

googlebot added CLA: yes and removed CLA: no labels Jun 28, 2016

Adam-Meisen force-pushed the master branch from 7b7e96d to d71ee0d Compare June 28, 2016 01:39

Adam-Meisen closed this Jun 28, 2016

Adam-Meisen reopened this Jun 28, 2016

Adam-Meisen force-pushed the master branch from d71ee0d to 4e7082d Compare June 28, 2016 02:20

Adam-Meisen force-pushed the master branch from 4e7082d to af19371 Compare June 28, 2016 08:13

Adam-Meisen changed the title ~~Fix for Issue#1750, move styles from sample.css to heroes.component.css~~ docs(toh-6) move styles from sample.css to heroes.component.css Jun 28, 2016

Adam-Meisen force-pushed the master branch 2 times, most recently from 37fc5ae to 0940692 Compare June 28, 2016 21:03

chalin reviewed Jun 29, 2016
View reviewed changes

Adam-Meisen force-pushed the master branch from 5b43b1d to a786d3c Compare June 29, 2016 21:23

Adam-Meisen force-pushed the master branch 2 times, most recently from acbb556 to 3840c01 Compare July 9, 2016 06:38

Adam-Meisen force-pushed the master branch from 3840c01 to 04fa71f Compare August 1, 2016 21:06

chalin mentioned this pull request Aug 2, 2016

docs(toh-6): add hero search to Dart; minor edits to TS #2018

Merged

Adam-Meisen force-pushed the master branch from 04fa71f to b8462f9 Compare August 2, 2016 05:57

Adam-Meisen reviewed Aug 2, 2016
View reviewed changes

Adam-Meisen force-pushed the master branch from b8462f9 to 0320013 Compare August 2, 2016 16:33

Adam-Meisen reviewed Aug 2, 2016
View reviewed changes

chalin reviewed Aug 2, 2016
View reviewed changes

Adam-Meisen force-pushed the master branch from 0320013 to 48059ed Compare August 2, 2016 19:52

Adam-Meisen reviewed Aug 2, 2016
View reviewed changes

wardbell added the pr_state: LGTM label Aug 5, 2016

wardbell closed this in d26afa9 Aug 5, 2016

Adam-Meisen mentioned this pull request Aug 5, 2016

Sample.css is used in the examples but never mentioned in the tutorial #1750

Closed

		@@ -396,7 +398,6 @@ block observables-section
		We start by creating `HeroSearchService` that sends search queries to our server's web api.

		+makeExample('toh-6/ts/app/hero-search.service.ts', null, 'app/hero-search.service.ts')(format=".")

docs(toh-6) move styles from sample.css to heroes.component.css #1751

docs(toh-6) move styles from sample.css to heroes.component.css #1751

Uh oh!

Conversation

Adam-Meisen commented Jun 26, 2016

Uh oh!

googlebot commented Jun 26, 2016

Uh oh!

googlebot commented Jun 26, 2016

Uh oh!

Adam-Meisen commented Jun 26, 2016

verify. Thanks.

Uh oh!

googlebot commented Jun 26, 2016

Uh oh!

googlebot commented Jun 26, 2016

Uh oh!

googlebot commented Jun 28, 2016

Uh oh!

googlebot commented Jun 28, 2016

Uh oh!

googlebot commented Jun 28, 2016

Uh oh!

googlebot commented Jun 28, 2016

Uh oh!

Adam-Meisen commented Jun 28, 2016

Uh oh!

Adam-Meisen commented Jun 28, 2016

Uh oh!

Adam-Meisen commented Jun 28, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Foxandxss commented Jun 28, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin commented Jun 29, 2016

Uh oh!

Adam-Meisen commented Jun 29, 2016

Uh oh!

chalin commented Aug 1, 2016

Uh oh!

chalin commented Aug 1, 2016

Uh oh!

chalin commented Aug 1, 2016

Uh oh!

Adam-Meisen commented Aug 1, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin commented Aug 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin commented Aug 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

+makeExample('app/heroes.component.html', 'delete')

Uh oh!

Adam-Meisen commented Aug 2, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chalin commented Aug 2, 2016

Uh oh!

wardbell commented Aug 5, 2016

Uh oh!

Uh oh!

Adam-Meisen commented Jun 28, 2016 •

edited

Loading