Skip to content

Commit 7209fcd

Browse files
fredsafredsa
committed
TODOs
1 parent c7bc715 commit 7209fcd

4 files changed

Lines changed: 30 additions & 0 deletions

File tree

‎app/js/controllers.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,10 +35,12 @@ function PageController($scope, $http, DoSerial, $routeParams, $window) {
3535
($scope.config && $scope.config.playground_namespace);
3636
};
3737

38+
// TODO: determine if there's a better way
3839
$scope.datastore_admin = function() {
3940
$window.open('/playground/datastore/' + $scope.namespace(), '_blank');
4041
};
4142

43+
// TODO: determine if there's a better way
4244
$scope.memcache_admin = function() {
4345
$window.open('/playground/memcache/' + $scope.namespace(), '_blank');
4446
};
@@ -118,6 +120,7 @@ function ProjectController($scope, $browser, $http, $routeParams, $window,
118120
$scope.url_of(file) : '';
119121
};
120122

123+
// TODO: don't expose function on $scope, while retaining testability
121124
$scope._get = function(file, success_cb) {
122125
if (file.hasOwnProperty('contents')) {
123126
success_cb();
@@ -161,6 +164,7 @@ function ProjectController($scope, $browser, $http, $routeParams, $window,
161164
*/
162165
};
163166

167+
// TODO: consider replacing DOM maniupulation here with a directive
164168
$scope.create_editor = function(mime_type) {
165169
if ($scope._editor) {
166170
angular.element($scope._editor.getWrapperElement()).remove();
@@ -232,6 +236,7 @@ function PageController($scope, $http, $location, $routeParams, $window,
232236
});
233237
};
234238
239+
// TODO: don't use prompt()
235240
$scope.prompt_to_delete_project = function(project) {
236241
var answer = prompt("Are you sure you want to delete project " +
237242
project.name + "?\nType 'yes' to confirm.", "no");
@@ -262,6 +267,7 @@ function PageController($scope, $http, $location, $routeParams, $window,
262267
263268
function LightboxController($scope, $window) {
264269
270+
// TODO: determine if there's a better way
265271
$scope.reload = function() {
266272
$window.location.reload();
267273
};
@@ -276,6 +282,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
276282
Backoff, DoSerial, DomElementById,
277283
WrappedElementById) {
278284
285+
// TODO: determine if there's a better way
279286
var source_image = WrappedElementById('source-image');
280287
281288
// { "app.yaml" : {
@@ -309,6 +316,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
309316
_saveDirtyFiles();
310317
})
311318
.then(function() {
319+
// TODO: try to avoid DOM access
312320
var container = WrappedElementById('output-container');
313321
if (_output_window && _output_window.closed) {
314322
_popout = false;
@@ -342,6 +350,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
342350
.error(function(data, status, headers, config) {
343351
$scope.filestatus = 'Failed to save ' + path;
344352
file.dirty = true;
353+
// implement seconds() function
345354
var secs = Backoff.backoff() / 1000;
346355
$log.warn(path, 'failed to save; will retry in', secs, 'secs');
347356
Backoff.schedule(_saveDirtyFiles);
@@ -361,6 +370,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
361370
}
362371
}
363372
373+
// TODO: don't use prompt()
364374
$scope.prompt_file_delete = function() {
365375
var answer = prompt("Are you sure you want to delete " +
366376
$scope.current_file.name + "?\nType 'yes' to confirm.",
@@ -371,6 +381,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
371381
$scope.deletefile($scope.current_file);
372382
}
373383
384+
// TODO: don't use prompt()
374385
$scope.prompt_project_rename = function(project) {
375386
var new_project_name = prompt('Enter a new name for this project',
376387
project.name);
@@ -391,6 +402,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
391402
});
392403
}
393404
405+
// TODO: don't use prompt()
394406
$scope.prompt_file_rename = function() {
395407
var new_filename = prompt(
396408
'New filename?\n(You may specify a full path such as: foo/bar.txt)',
@@ -407,17 +419,20 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
407419
$scope.movefile($scope.current_file, new_filename);
408420
}
409421
422+
// TODO: determine if there's a better way
410423
function hide_context_menus() {
411424
$scope.showfilecontextmenu = false;
412425
$scope.showprojectcontextmenu = false;
413426
}
414427
415428
// setup context menu clear handler
429+
// TODO: use $window rather than window
416430
window.addEventListener('click', function(evt) {
417431
hide_context_menus();
418432
$scope.$apply();
419433
}, false);
420434
435+
// TODO: avoid DOM access
421436
$scope.project_context_menu = function(evt) {
422437
evt.stopPropagation();
423438
hide_context_menus();
@@ -427,6 +442,7 @@ function ProjectController($scope, $http, $filter, $log, $timeout, $routeParams,
427442
menuDiv.css('top', evt.pageY + 'px');
428443
};
429444
445+
// TODO: avoid DOM access
430446
$scope.file_context_menu = function(evt, file) {
431447
evt.stopPropagation();
432448
hide_context_menus();

‎app/js/services.js‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
angular.module('playgroundApp.services', [])
66

7+
// TODO: improve upon flushDoSerial(); allow one step to be executed at a time
78
.factory('DoSerial', function($q, $timeout, $log) {
89

910
var deferred = $q.defer();
@@ -64,12 +65,14 @@ angular.module('playgroundApp.services', [])
6465
};
6566
})
6667

68+
// TODO: determine if there's a better way
6769
.factory('DomElementById', function($window) {
6870
return function(id) {
6971
return $window.document.getElementById(id);
7072
};
7173
})
7274

75+
// TODO: determine if there's a better way
7376
.factory('WrappedElementById', function(DomElementById) {
7477
return function(id) {
7578
return angular.element(DomElementById(id));
@@ -78,6 +81,7 @@ angular.module('playgroundApp.services', [])
7881

7982
/*
8083
84+
// TODO: determine if there's a better way
8185
.factory('Backoff', function($timeout) {
8286
8387
"Exponential backoff service."

‎test/unit/controllersSpec.js‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ describe('ProjectController', function() {
9292
var findable_elements;
9393

9494
beforeEach(module(function($provide) {
95+
// TODO: there has to be a better way
9596
$provide.factory('$window', function() {
9697
var $window = {
9798
location: { replace: jasmine.createSpy(),
@@ -121,12 +122,15 @@ describe('ProjectController', function() {
121122

122123
beforeEach(inject(function($rootScope, $injector, $window) {
123124
scope = $rootScope.$new();
125+
126+
// TODO: determine how we should test when this controller relies on scope initialized by a parent controller
124127
// TODO: remove if we instead instantiate a PageController
125128
scope.config = {
126129
'PLAYGROUND_USER_CONTENT_HOST': 'localhost:9100',
127130
};
128131
$httpBackend = $injector.get('$httpBackend');
129132

133+
// TODO: determine if there's a better way to test JavaScript functions which are expected to exist thanks to script tags
130134
$window.CodeMirror = jasmine.createSpy('CodeMirror').andReturn(
131135
jasmine.createSpyObj('CodeMirror', ['getWrapperElement', 'setValue', 'setOption', 'focus'])
132136
);
@@ -159,6 +163,7 @@ describe('ProjectController', function() {
159163

160164
describe('initialization', function() {
161165

166+
// TODO: determine if there's a better way to ensure that $routeParams is populated
162167
it('should set $scope.project to the project identified by $routeParams.project_id', inject(function($routeParams) {
163168
$routeParams.project_id = 42;
164169
var project = make_project(42);
@@ -232,6 +237,7 @@ describe('ProjectController', function() {
232237
it('should return //localhost:9100/p/:project_id/getfile/:filename', inject(function($window) {
233238
$window.location.pathname = '/playground/p/42/';
234239
var png = make_file('logo.png', 'image/png');
240+
// TODO: try to avoid localhost:9100
235241
expect(scope.url_of(png)).toEqual('//localhost:9100/playground/p/42/getfile/logo.png');
236242
}));
237243

@@ -436,6 +442,7 @@ describe('ProjectController', function() {
436442
expect(scope.current_file).toEqual(make_text_file());
437443
});
438444

445+
// TODO: test order of calls to DoSerial functions
439446
it('should call DoSerial.then() and DoSerial.tick()', inject(function(DoSerial) {
440447
spyOn(DoSerial, 'then').andCallThrough();
441448
spyOn(DoSerial, 'tick').andCallThrough();

‎test/unit/servicesSpec.js‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ describe('service', function() {
1010
describe('playgroundHttpInterceptor', function() {
1111

1212
it('should return HTTP normal responses unmodified', inject(function(playgroundHttpInterceptor) {
13+
// TODO: use jasmine spy instead
1314
var called = false;
1415
var http_promise = {
1516
then: function(success_fn, error_fn) {
@@ -124,6 +125,7 @@ describe('service', function() {
124125

125126
it('should log and continue after exception', function() {
126127

128+
// TODO: determine better way to test with $exceptionHandlerProvider
127129
module(function($exceptionHandlerProvider) {
128130
    $exceptionHandlerProvider.mode('log');
129131
  });
@@ -144,6 +146,7 @@ describe('service', function() {
144146

145147
});
146148

149+
// TODO: determine if there's a better way to test window / document stuff
147150
describe('DomElementById', function() {
148151

149152
it('should call $window.document.getElementById(:id)', inject(function($window, DomElementById) {

0 commit comments

Comments
 (0)