Skip to content
This repository was archived by the owner on Feb 26, 2024. It is now read-only.

chore(structure): WIP refactor to Angular style guide conventions #289

Closed
wants to merge 18 commits into from

Conversation

teropa
Copy link

@teropa teropa commented Dec 11, 2015

Reorganize application code to be more closely aligned
with the Angular Style Guide.

This is for the final version of the app and hasn’t been
factored into steps or into the tutorial text.

  1. Folders-by-feature structure: /core, /phone_list, /phone_detail
    https://github.com/johnpapa/angular-styleguide#style-y152
  2. App root module depends on feature modules and core module.
    https://github.com/johnpapa/angular-styleguide#style-y165
  3. One component per file. Each controller/factory/filter in its own file.
    https://github.com/johnpapa/angular-styleguide#style-y001
  4. Feature+type file naming convention.
    https://github.com/johnpapa/angular-styleguide#style-y120
  5. Module files, *.module.js for each module.
    https://github.com/johnpapa/angular-styleguide#style-y127
  6. Component registration with getters
    https://github.com/johnpapa/angular-styleguide#style-y022
  7. Named functions
    https://github.com/johnpapa/angular-styleguide#style-y024
  8. Manual injection
    https://github.com/johnpapa/angular-styleguide#style-y091

Related to angular/angular.js#13312
Related to angular/angular.js#13834

@teropa
Copy link
Author

teropa commented Dec 11, 2015

@petebacondarwin Here's a more styleguide-compatible version of the final PhoneCat app. I'd love to hear your opinions on a couple of things:

  1. Do you think it would make sense to teach this kind of app structure in the tutorial?
  2. Irrespective of (1), would you be willing to include this (or something like this) as a separate branch in the PhoneCat repo so that people who want to follow along with the ng-upgrade tutorial can use it as a starting point?

@petebacondarwin
Copy link
Contributor

I like where this is going. I'll try to do a proper review over the weekend.
Can we consider using component directives rather than controllers?

@teropa
Copy link
Author

teropa commented Dec 11, 2015

Great, thanks!

Can we consider using component directives rather than controllers?

Yes, I think so! Just pushed a commit that attempts it.

Caveat: Ideally this would use the component router, but not sure if that's doable at this point. Instead using the old router with little template snippets that instantiate the component directives.

Caveat 2: Haven't seen what @johnpapa's style guide for components will look like yet :)

teropa and others added 17 commits January 24, 2016 13:09
- add ngApp directive to bootstrap the app
- add simple template with an expression
- Added static html list with two phones into index.html
- Converted the static html list into dynamic one by:
  - creating phoneList component and PhoneListCtrl controller for the application
  - extracting the data from HTML into a the controller as an in-memory
    dataset
  - converting the static document into a template with the use of
    `[ngRepeat]` [directive] which iterates over the dataset with phones,
    clones the ngRepeat template for each instance and renders it into the
    view
- Added a simple unit test to show off how to write tests and run them
  with Karma (see README.md for instructions)
- Added a search box to demonstrate how:
  - the data-binding works on input fields
  - to use [filter] filter
  - [ngRepeat] automatically shrinks and grows the number of phones in the view
- Added an end-to-end test to:
  - show how end-to-end tests are written and used
  - to prove that the search box and the repeater are correctly wired together
- Add "age" property to the phone model
- Add select box to control phone list order
- Override the default order value in controller
- Add unit and e2e test for this feature

Closes angular#213
- Replaced the in-memory dataset with data loaded from the server (in
  the form of static phone.json file to make this tutorial backend
  agnostic)
  - The json file is loaded using the [$http] service
- Demonstrate the use of [services][service] and [dependency injection][DI]
  - The [$http] is injected into the controller through [dependency injection][DI]

(Added inline annotation - thanks to @guatebus - closes angular#207)
- adding phone image and links to phone pages
- add end2end test that verifies our phone links
- css to style the page just a notch
- Introduce the [$route] service which allows binding URLs for deep-linking with
  views
  - Load the ngRoute module
  - Map `/phones' to the phoneList component
  - Map `/phones/<phone-id>' to the phoneDetail component
  - Replace content of index.html with [ngView] directive

- Create phone list route
  - Preserve existing phoneList component
- Create phone details route
  - Empty placeholder phoneDetails component
- Fetch data for and render phone detail view
  - PhoneDetailCtrl controller to fetch details json with [$http] for a specific
    phone
  - template for the phone detailed view
- CSS to make the phone details page look "pretty"
- Added custom checkmark filter
- Update phone detail template to use checkmark filter
- Added spec for the filter
In the phone detail view, clicking on a thumbnail image, changes the
main phone image to be the large version of the thumbnail image.

- Define mainImageUrl model variable in the PhoneDetailCtrl and set its
  default value
- Create setImage controller method to change mainImageUrl
- Register ng:click handler for thumb images to use setImage controller
  method
- Add e2e tests for this feature
- Add css to change the mouse cursor when user points at thumnail images
- Replaced [$http] with [$resource]
- Created a custom Phone service that represents the $resource client
    - Organized code into modules by feature structure
    - Split code so that one component is introduced in each file
    - Switched to feature.type.js file naming scheme
@googlebot
Copy link

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.

@Hotell
Copy link

Hotell commented Feb 21, 2016

👍

@Hotell
Copy link

Hotell commented Feb 21, 2016

I would also recommend to get rid of ng-app and use manual bootstrap angular.bootstrap(...)

and encapsulate

<div class="view-container">
    <div ng-view class="view-frame"></div>
</div>

into an root component

<phone-cat-app></phone-cat-app>

@petebacondarwin
Copy link
Contributor

@Hotell that's not a bad idea either.

@Hotell
Copy link

Hotell commented Feb 22, 2016

I'll gladly help with this, because I wanna use this PR as a starting point for showcasing people how to migrate to Typescript + ngMetadata and then leveraging ngUpgrade to ng2

gkalpak added a commit to gkalpak/angular-phonecat that referenced this pull request Apr 11, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.

--
This was inspired by (and loosely based on) angular#289.
Mad props to @teropa for leading the way :)

Fixes angular#198
Fixes angular#214
Fixes angular#224
Fixes angular#230
Fixes angular#243
Fixes angular#246
Fixes angular#252
Fixes angular#277
Fixes angular#286
Fixes angular#295
Fixes angular#303
Fixes angular#304
Fixes angular#323
Fixes angular#324

Closes angular#268
Closes angular#270
Closes angular#278
Closes angular#280
Closes angular#289
Closes angular#309
Closes angular#311
Closes angular#319
gkalpak added a commit to gkalpak/angular-phonecat that referenced this pull request Apr 11, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

--
This was inspired by (and loosely based on) angular#289.
Mad props to @teropa for leading the way :)

--
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved in the `pre-v1.5.0-snapshot` branch. The `v1.4.x` version of the tutorial should be pointed
to that branch instead of `master`.

Fixes angular#198
Fixes angular#214
Fixes angular#224
Fixes angular#230
Fixes angular#243
Fixes angular#246
Fixes angular#252
Fixes angular#277
Fixes angular#286
Fixes angular#295
Fixes angular#303
Fixes angular#304
Fixes angular#323
Fixes angular#324

Closes angular#268
Closes angular#270
Closes angular#278
Closes angular#280
Closes angular#289
Closes angular#309
Closes angular#311
Closes angular#319
gkalpak added a commit to gkalpak/angular-phonecat that referenced this pull request Apr 11, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, I have tested with the following versions on Node (on Windows 10) and everything seems to work fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) angular#289.
Mad props to @teropa for leading the way :)

--
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved in the `pre-v1.5.0-snapshot` branch. The `v1.4.x` version of the tutorial should be pointed
to that branch instead of `master`.

Fixes angular#198
Fixes angular#214
Fixes angular#224
Fixes angular#230
Fixes angular#243
Fixes angular#246
Fixes angular#252
Fixes angular#277
Fixes angular#286
Fixes angular#295
Fixes angular#303
Fixes angular#304
Fixes angular#323
Fixes angular#324

Closes angular#268
Closes angular#270
Closes angular#278
Closes angular#280
Closes angular#289
Closes angular#309
Closes angular#311
Closes angular#319
gkalpak added a commit to gkalpak/angular-phonecat that referenced this pull request Apr 12, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process, several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, this has been tested with the following versions of Node (on Windows 10) and everything worked
fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) angular#289.
Mad props to @teropa for leading the way :)

--
**Note:**
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved on the `pre-v1.5.0-snapshot` branch. The `v1.4.x` version of the tutorial should be pointed
to that branch instead of `master`.

--
Related to angular/angular.js#14416.
Related to angular/angular-seed#329.
Related to angular/angular-seed#333.

---
Fixes angular#198
Fixes angular#214
Fixes angular#224
Fixes angular#230
Fixes angular#243
Fixes angular#246
Fixes angular#252
Fixes angular#277
Fixes angular#286
Fixes angular#295
Fixes angular#303
Fixes angular#304
Fixes angular#323
Fixes angular#324

Closes angular#268
Closes angular#270
Closes angular#278
Closes angular#280
Closes angular#289
Closes angular#309
Closes angular#311
Closes angular#319
@gkalpak gkalpak closed this in #326 May 24, 2016
gkalpak added a commit that referenced this pull request May 24, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process, several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, this has been tested with the following versions of Node (on Windows 10) and everything worked
fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) #289.
Mad props to @teropa for leading the way :)

--
**Note:**
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved on the `pre-v1.5.0-snapshot` branch. The `v1.4.x` version of the tutorial should be pointed
to that branch instead of `master`.

--
Related to angular/angular.js#14416.
Related to angular/angular-seed#329.
Related to angular/angular-seed#333.

---
Fixes #198
Fixes #214
Fixes #224
Fixes #230
Fixes #243
Fixes #246
Fixes #252
Fixes #277
Fixes #286
Fixes #295
Fixes #303
Fixes #304
Fixes #323
Fixes #324

Closes #268
Closes #270
Closes #278
Closes #280
Closes #289
Closes #309
Closes #311
Closes #319
gkalpak added a commit that referenced this pull request May 24, 2016
…s for the post-1.5.0 era

This commit and the accompanying updates to the tutorial steps constitute a major re-structuring of
the tutorial app's codebase, aiming at applying established best practices (in terms of file
naming/layout and code organization) and utilizing several new features and enhancements (most
notably components) introduced in recent versions of Angular (especially v1.5).

Apart from the overall changes, two new chapters were introduced: one on components and one on code
organization.

--
In the process, several other things were (incidentally) taken care of, including:

* Dependencies were upgraded to latest versions.
* Animations were polished.
* Outdated links were updated.
* The app's base URL was changed to `/` (instead of `/app/`).

BTW, this has been tested with the following versions of Node (on Windows 10) and everything worked
fine:

* 0.11.16
* 4.2.6
* 4.4.2
* 5.10.0

--
This was inspired by (and loosely based on) #289.
Mad props to @teropa for leading the way :)

--
**Note:**
The old version of the tutorial, that is compatible with Angular version 1.4 or older, has been
saved on the `pre-v1.5.0-snapshot` branch. The `v1.4.x` version of the tutorial should be pointed
to that branch instead of `master`.

--
Related to angular/angular.js#14416.
Related to angular/angular-seed#329.
Related to angular/angular-seed#333.

---
Fixes #198
Fixes #214
Fixes #224
Fixes #230
Fixes #243
Fixes #246
Fixes #252
Fixes #277
Fixes #286
Fixes #295
Fixes #303
Fixes #304
Fixes #323
Fixes #324

Closes #268
Closes #270
Closes #278
Closes #280
Closes #289
Closes #309
Closes #311
Closes #319
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants