Commit 4f4f5d13 authored by Robert Knight's avatar Robert Knight

Rename `createLoginPopupWindow` to `openAuthPopupWindow`

 - Use the term "auth" for consistency with the `authorize` method and use
   the verb "open" to clarify that the popup window is being both created
   and shown.

 - Clarify the relation of the `$window` parameter to the returned
   window.
parent 7626dd65
...@@ -255,7 +255,7 @@ function auth($http, $rootScope, $window, OAuthClient, ...@@ -255,7 +255,7 @@ function auth($http, $rootScope, $window, OAuthClient,
* then exchange for access and refresh tokens. * then exchange for access and refresh tokens.
*/ */
function login() { function login() {
var authWindow = OAuthClient.createLoginPopupWindow($window); var authWindow = OAuthClient.openAuthPopupWindow($window);
return oauthClient().then(client => { return oauthClient().then(client => {
return client.authorize($window, authWindow); return client.authorize($window, authWindow);
}).then(code => { }).then(code => {
......
...@@ -89,7 +89,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -89,7 +89,7 @@ describe('sidebar.oauth-auth', function () {
fakeClient.config = config; fakeClient.config = config;
return fakeClient; return fakeClient;
}; };
FakeOAuthClient.createLoginPopupWindow = sinon.stub(); FakeOAuthClient.openAuthPopupWindow = sinon.stub();
fakeWindow = new FakeWindow; fakeWindow = new FakeWindow;
...@@ -504,7 +504,7 @@ describe('sidebar.oauth-auth', function () { ...@@ -504,7 +504,7 @@ describe('sidebar.oauth-auth', function () {
it('calls OAuthClient#authorize', () => { it('calls OAuthClient#authorize', () => {
var fakePopup = {}; var fakePopup = {};
FakeOAuthClient.createLoginPopupWindow.returns(fakePopup); FakeOAuthClient.openAuthPopupWindow.returns(fakePopup);
return auth.login().then(() => { return auth.login().then(() => {
assert.calledWith(fakeClient.authorize, fakeWindow, fakePopup); assert.calledWith(fakeClient.authorize, fakeWindow, fakePopup);
}); });
......
...@@ -153,7 +153,7 @@ class OAuthClient { ...@@ -153,7 +153,7 @@ class OAuthClient {
* *
* @param {Window} $window - Window which will receive the auth response. * @param {Window} $window - Window which will receive the auth response.
* @param {Window} authWindow - Popup window where the login prompt will be shown. * @param {Window} authWindow - Popup window where the login prompt will be shown.
* This should be created using `createLoginPopupWindow`. * This should be created using `openAuthPopupWindow`.
* @return {Promise<string>} * @return {Promise<string>}
*/ */
authorize($window, authWindow) { authorize($window, authWindow) {
...@@ -214,17 +214,17 @@ class OAuthClient { ...@@ -214,17 +214,17 @@ class OAuthClient {
} }
/** /**
* Create a pop-up window for use with `OAuthClient#authorize`. * Create and show a pop-up window for use with `OAuthClient#authorize`.
* *
* This function _must_ be called in the same turn of the event loop as the * This function _must_ be called in the same turn of the event loop as the
* button or link which initiates login to avoid triggering the popup blocker * button or link which initiates login to avoid triggering the popup blocker
* in certain browsers. See https://github.com/hypothesis/client/issues/534 * in certain browsers. See https://github.com/hypothesis/client/issues/534
* and https://github.com/hypothesis/client/issues/535. * and https://github.com/hypothesis/client/issues/535.
* *
* @param {Window} $window - The parent of the popup window. * @param {Window} $window - The parent of the created window.
* @return {Window} * @return {Window} The new popup window.
*/ */
static createLoginPopupWindow($window) { static openAuthPopupWindow($window) {
// In Chrome & Firefox the sizes passed to `window.open` are used for the // In Chrome & Firefox the sizes passed to `window.open` are used for the
// viewport size. In Safari the size is used for the window size including // viewport size. In Safari the size is used for the window size including
// title bar etc. There is enough vertical space at the bottom to allow for // title bar etc. There is enough vertical space at the bottom to allow for
......
...@@ -162,10 +162,10 @@ describe('sidebar.util.oauth-client', () => { ...@@ -162,10 +162,10 @@ describe('sidebar.util.oauth-client', () => {
}); });
}); });
describe('.createLoginPopupWindow', () => { describe('.openAuthPopupWindow', () => {
it('creates and returns the popup window', () => { it('opens a popup window', () => {
var fakeWindow = new FakeWindow; var fakeWindow = new FakeWindow;
var popupWindow = OAuthClient.createLoginPopupWindow(fakeWindow); var popupWindow = OAuthClient.openAuthPopupWindow(fakeWindow);
assert.equal(popupWindow, fakeWindow.open.returnValues[0]); assert.equal(popupWindow, fakeWindow.open.returnValues[0]);
assert.calledWith( assert.calledWith(
fakeWindow.open, fakeWindow.open,
...@@ -184,7 +184,7 @@ describe('sidebar.util.oauth-client', () => { ...@@ -184,7 +184,7 @@ describe('sidebar.util.oauth-client', () => {
}); });
function authorize() { function authorize() {
var popupWindow = OAuthClient.createLoginPopupWindow(fakeWindow); var popupWindow = OAuthClient.openAuthPopupWindow(fakeWindow);
var authorized = client.authorize(fakeWindow, popupWindow); var authorized = client.authorize(fakeWindow, popupWindow);
return { authorized, popupWindow }; return { authorized, popupWindow };
} }
......
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