Commit f2fcb297 authored by Robert Knight's avatar Robert Knight

Remove usage of bindings in component constructors

In Angular 1.6 the behavior where inputs to components ("bindings")
were accessible in the controller's constructor was deprecated [1] but a
switch was made available to re-enable the old behavior to assist migration.
This switch has been removed in Angular 1.7, so we need to stop relying
on it before we can upgrade.

The correct thing to do is to put all init logic which accesses bindings
(`this.someBindingName`) in the `$onInit` lifecycle method.

This commit makes the minimal amount of changes to enable the app and
tests to run without requiring pre-assigned bindings.

[1] https://toddmotto.com/angular-1-6-is-here
parent b75882bf
......@@ -33,8 +33,10 @@ function AnnotationViewerContentController (
const id = $routeParams.id;
this.search.update = function (query) {
$location.path('/stream').search('q', query);
this.$onInit = () => {
this.search.update = function (query) {
$location.path('/stream').search('q', query);
};
};
store.subscribe(function () {
......
......@@ -99,7 +99,7 @@ function AnnotationController(
* All initialization code except for assigning the controller instance's
* methods goes here.
*/
function init() {
this.$onInit = () => {
/** Determines whether controls to expand/collapse the annotation body
* are displayed adjacent to the tags field.
*/
......@@ -156,7 +156,7 @@ function AnnotationController(
self.edit();
}
}
}
};
/** Save this annotation if it's a new highlight.
*
......@@ -569,8 +569,6 @@ function AnnotationController(
}
return self.group().type !== 'private';
};
init();
}
module.exports = {
......
......@@ -202,10 +202,12 @@ function SidebarContentController(
streamer.connect();
});
// If the user is logged in, we connect nevertheless
if (this.auth.status === 'logged-in') {
streamer.connect();
}
this.$onInit = () => {
// If the user is logged in, we connect nevertheless
if (this.auth.status === 'logged-in') {
streamer.connect();
}
};
$scope.$on(events.USER_CHANGED, function () {
streamer.reconnect();
......
......@@ -52,15 +52,6 @@ function StreamContentController(
store.setForceVisible(id, true);
};
Object.assign(this.search, {
query: function () {
return $routeParams.q || '';
},
update: function (q) {
$location.search({q: q});
},
});
store.subscribe(function () {
self.rootThread = rootThread.thread(store.getState());
});
......@@ -69,6 +60,11 @@ function StreamContentController(
store.setSortKey('Newest');
this.loadMore = fetch;
this.$onInit = () => {
this.search.query = () => $routeParams.q || '';
this.search.update = q => $location.search({ q });
};
}
module.exports = {
......
......@@ -16,10 +16,12 @@ const icons = {
// @ngInject
function SvgIconController($element) {
if (!icons[this.name]) {
throw new Error('Unknown icon: ' + this.name);
}
$element[0].innerHTML = icons[this.name];
this.$onInit = () => {
if (!icons[this.name]) {
throw new Error('Unknown icon: ' + this.name);
}
$element[0].innerHTML = icons[this.name];
};
}
module.exports = {
......
......@@ -151,8 +151,7 @@ describe('annotation', function() {
})
.component('markdown', {
bindings: require('../markdown').bindings,
})
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
});
});
beforeEach(angular.mock.module('h'));
......
......@@ -35,8 +35,7 @@ describe('annotationViewerContent', function () {
before(function () {
angular.module('h', [])
.component('annotationViewerContent',
require('../annotation-viewer-content'))
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
require('../annotation-viewer-content'));
});
beforeEach(angular.mock.module('h'));
......
......@@ -57,8 +57,7 @@ describe('sidebar.components.hypothesis-app', function () {
}));
angular.module('h', [])
.component('hypothesisApp', component)
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
.component('hypothesisApp', component);
});
beforeEach(angular.mock.module('h'));
......
......@@ -67,8 +67,7 @@ describe('sidebar.components.sidebar-content', function () {
angular: angular,
'../search-client': FakeSearchClient,
})
))
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
));
});
beforeEach(angular.mock.module('h'));
......
......@@ -26,8 +26,7 @@ describe('StreamContentController', function () {
before(function () {
angular.module('h', [])
.component('streamContent', require('../stream-content'))
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
.component('streamContent', require('../stream-content'));
});
beforeEach(function () {
......
......@@ -7,8 +7,7 @@ const util = require('../../directive/test/util');
describe('svgIcon', function () {
before(function () {
angular.module('app', [])
.component('svgIcon', require('../svg-icon'))
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
.component('svgIcon', require('../svg-icon'));
});
beforeEach(function () {
......
......@@ -115,8 +115,7 @@ describe('threadList', function () {
before(function () {
angular.module('app', [])
.component('threadList', threadList)
.config(($compileProvider) => $compileProvider.preAssignBindingsEnabled(true));
.component('threadList', threadList);
});
beforeEach(function () {
......
......@@ -70,26 +70,29 @@ function ThreadListController($element, $scope, settings, VirtualThreadList) {
const options = Object.assign({
scrollRoot: this.scrollRoot,
}, virtualThreadOptions);
const visibleThreads = new VirtualThreadList($scope, window, this.thread, options);
visibleThreads.on('changed', function (state) {
self.virtualThreadList = {
visibleThreads: state.visibleThreads,
invisibleThreads: state.invisibleThreads,
offscreenUpperHeight: state.offscreenUpperHeight + 'px',
offscreenLowerHeight: state.offscreenLowerHeight + 'px',
};
scopeTimeout($scope, function () {
state.visibleThreads.forEach(function (thread) {
const height = getThreadHeight(thread.id);
if (!height) {
return;
}
visibleThreads.setThreadHeight(thread.id, height);
});
}, 50);
});
let visibleThreads;
this.$onInit = () => {
visibleThreads = new VirtualThreadList($scope, window, this.thread, options);
visibleThreads.on('changed', function (state) {
self.virtualThreadList = {
visibleThreads: state.visibleThreads,
invisibleThreads: state.invisibleThreads,
offscreenUpperHeight: state.offscreenUpperHeight + 'px',
offscreenLowerHeight: state.offscreenLowerHeight + 'px',
};
scopeTimeout($scope, function () {
state.visibleThreads.forEach(function (thread) {
const height = getThreadHeight(thread.id);
if (!height) {
return;
}
visibleThreads.setThreadHeight(thread.id, height);
});
}, 50);
});
};
/**
* Return the vertical scroll offset for the document in order to position the
......@@ -134,7 +137,7 @@ function ThreadListController($element, $scope, settings, VirtualThreadList) {
});
this.$onChanges = function (changes) {
if (changes.thread) {
if (changes.thread && visibleThreads) {
visibleThreads.setRootThread(changes.thread.currentValue);
}
};
......
......@@ -95,19 +95,6 @@ function configureToastr(toastrConfig) {
});
}
// @ngInject
function configureCompile($compileProvider) {
// Make component bindings available in controller constructor. When
// pre-assigned bindings is off, as it is by default in Angular >= 1.6.0,
// bindings are only available during and after the controller's `$onInit`
// method.
//
// This migration helper is being removed in Angular 1.7.0. To see which
// components need updating, look for uses of `preAssignBindingsEnabled` in
// tests.
$compileProvider.preAssignBindingsEnabled(true);
}
// @ngInject
function setupHttp($http, streamer) {
$http.defaults.headers.common['X-Client-Id'] = streamer.clientId;
......@@ -213,7 +200,6 @@ function startAngularApp(config) {
.value('time', require('./util/time'))
.value('urlEncodeFilter', require('./filter/url').encode)
.config(configureCompile)
.config(configureLocation)
.config(configureRoutes)
.config(configureToastr)
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment