4
\$\begingroup\$

In looking for a bit of feedback about an Angular app I have created.

I'm new to angular I want to clean up the loose ends before I carry on and add new functionality.

Below is the code snippet of index.html which includes custom directives that includes a block of HTML and various angular elements

It also includes a $http service to fetch Json from an external file

<body ng-controller="StoreController as store">
<script type="text/javascript" src="angular.js"></script>
<script type="text/javascript" src="app.js"></script>
<script type="text/javascript" src="product.directives.js"></script>

<h1 class="text-center">{{store.title}}</h1>
<ul class="list-group container">
    <li class="list-group-item col-sm-12" ng-repeat="product in store.products ">
        <h1>
            <product-title></product-title>
        </h1>
        <img ng-src="{{product.images[0].full}}" />
        <p ng-hide="product.canPurchase">Item is not available for purchase</p>
        <button ng-show="product.canPurchase">Add to cart</button>
        <product-panels></product-panels>
    </li>
</ul>
</body>

This is my app.js which contains the controllers

var app = angular.module('store', ['store-products']);

app.controller("ReviewController", function () {
// New review array slot is created and assigned blank
this.review = {};

// Form triggers function to push new review slot into array
this.addReview = function (product) {
    product.reviews.push(this.review);
    // Set the review form to blank again
    this.review = {};
}
});

app.controller('StoreController', ['$http', function ($http) {
this.title = "Angular App";
    console.log('test');
var store = this;
//store.products = [];

$http.get('/products.json').then(function (response) {
    console.log(response);
    store.products = response.data;
});
}]);

and finally here is my directives.js. with the logic being stored in external html files.

var app = angular.module('store-products', []);

app.directive('productPanels', function () {

return {
    restrict: 'E',
    templateUrl: 'product-panels.html',
    controller: function () {

        this.tab = 1;

        this.selectTab = function (setTab) {
            this.tab = setTab;
        }

        this.isSelected = function (checkTab) {
            return this.tab === checkTab;
        }

    },
    controllerAs: 'panel',
};
});

app.directive('productTitle', function () {

return {
    restrict: 'EA',
    templateUrl: 'product-title.html'
};
});
\$\endgroup\$
  • 1
    \$\begingroup\$ The link to app appears to yield a 404... can you correct that? \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Nov 14 '17 at 23:03
  • \$\begingroup\$ What is in product-panels.html? I presume product-title.html mostly displays the name/title of the product.... Does this plunker look similar to what your code looks like? Feel free to fork it, update it and add a link so we can have more context... \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Dec 15 '17 at 17:28
2
\$\begingroup\$

I noticed that you have a spot for controllers, but then in your directive you declare a function for the Controller Attribute instead of using a controller that you have already at your disposal from the controller's section.

I would think that you would want to be able to reuse that function that controls tab selection, you should create a controller for that so that you can reuse it on several pages, rather than write from scratch every time you create a page with Panel tabs.

\$\endgroup\$
  • \$\begingroup\$ It is difficult to know what the panels contains (presumably something important for each product...). I made a plunker and found that if the OP wanted the scope of each panel controller to be different, the scope: {} could be added to the return POJO for the directive... I also couldn't get the selectTab() functions to work unless I inject $scope to the controller \$\endgroup\$ – Sᴀᴍ Onᴇᴌᴀ Dec 15 '17 at 17:18
  • \$\begingroup\$ you should be able to inject the scope into a Controller that already exists. simple Panel functionality should be reusable. you should be able to call more than one controller in the Controller attribute of the return of the directive. so any extra functionality that is above the simple Panel Functionality should be in a separate controller \$\endgroup\$ – Malachi Dec 16 '17 at 13:45

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.