This repository has been archived by the owner on May 4, 2022. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 31
Reflect page title in document title #679
Merged
Merged
Changes from 28 commits
Commits
Show all changes
30 commits
Select commit
Hold shift + click to select a range
1fbd821
docs: clarify setTitle() with function-level doc
apetro 0a6caac
refactor: clarify setTitle() implementation
apetro 305e17b
fix: simplify setTitle() to handle app with same name as portal case
apetro 362399e
refactor: handle true branch first
apetro 57c16aa
docs: clarify inline comment
apetro 4ed9820
fix: handle edge case where theme lacks title
apetro f7aa6d1
fix: handle case where there isn't an active theme with a title
apetro 186864b
docs: copy edit inline comment
apetro f3b8182
refactor: build windowTitle from pieces
apetro e71dcb9
feat: support per-page window titles
apetro 83e8bde
test(main-service): unit test setTitle
apetro e2b15a9
fix(set-title): handle null or empty app name edge case
apetro ac6eb3a
refactor: order tests to match examples
apetro 3c078f3
test: describe tests conceptually rather than literally
apetro 6f3979a
test: match test cases with documented examples
apetro 61b4622
test: cover more setTitle edge cases
apetro 9794ad6
Merge branch 'master' into page-title-takes-string
apetro 8ff8529
refactor: split title computation from title setting
apetro e37ba9d
feat: app-header directive side effect sets document title
apetro dd505df
Merge branch 'master' into page-title-takes-string
apetro 9d07002
test: test that PortalMainController sets title to reflect theme
apetro fe6e427
test: note problematic specificity of document title setting test
apetro 19fcbb6
docs: JSDocs PortalMainController.updateTitle(pageTitle)
apetro 03698b2
test: test AppHeaderOptionsController updateTitle( pageTitle )
apetro 4af8ddb
docs: note frame-page directive document title setting side-effect
apetro bffb3a3
docs(changelog): note window title changes
apetro 8580be1
refactor: rename updateTitle to updateWindowTitle in Controller layer
apetro ae441b4
docs: note window title update side-effect in appHeader source comment
apetro 1ce21d2
Merge branch 'master' into page-title-takes-string
apetro 9590884
docs: use idiomatic JSDoc syntax for optional pageTitle
apetro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -33,6 +33,29 @@ define(['angular', 'require'], function(angular, require) { | |
layoutMode: 'list', // other option is 'widgets | ||
}; | ||
|
||
/** | ||
* Set Document title. | ||
* Asks mainService what the document title ought to be and | ||
* sets the document title to that value. | ||
* @param {string} pageTitle - Optional, name of specific page viewed. | ||
*/ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I see that we call updateTitle with no param later in this file. Maybe a comment on what happens when no param is called? What happens with the parameter? |
||
function updateWindowTitle(pageTitle) { | ||
var appTitle = NAMES.title; | ||
|
||
var portalTitle = ''; | ||
if ($rootScope.portal && $rootScope.portal.theme && | ||
$rootScope.portal.theme.title) { | ||
// there's an active theme with a title. | ||
// consider that title the name of the portal | ||
portalTitle = $rootScope.portal.theme.title; | ||
} | ||
|
||
var windowTitle = | ||
mainService.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
$document[0].title = windowTitle; | ||
} | ||
|
||
// =====functions ====== | ||
var init = function() { | ||
$scope.$storage = $localStorage.$default(defaults); | ||
|
@@ -44,13 +67,13 @@ define(['angular', 'require'], function(angular, require) { | |
$scope.APP_OPTIONS = APP_OPTIONS; | ||
|
||
if (NAMES.title) { | ||
mainService.setTitle(); | ||
updateWindowTitle(); | ||
} | ||
// https://github.com/Gillespie59/eslint-plugin-angular/issues/231 | ||
// eslint-disable-next-line angular/on-watch | ||
$rootScope.$watch('portal.theme', function(newValue, oldValue) { | ||
if (newValue && newValue !== oldValue) { | ||
mainService.setTitle(); | ||
updateWindowTitle(); | ||
} | ||
}); | ||
|
||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,222 @@ | ||
/* | ||
* Licensed to Apereo under one or more contributor license | ||
* agreements. See the NOTICE file distributed with this work | ||
* for additional information regarding copyright ownership. | ||
* Apereo licenses this file to you under the Apache License, | ||
* Version 2.0 (the "License"); you may not use this file | ||
* except in compliance with the License. You may obtain a | ||
* copy of the License at the following location: | ||
* | ||
* http://www.apache.org/licenses/LICENSE-2.0 | ||
* | ||
* Unless required by applicable law or agreed to in writing, | ||
* software distributed under the License is distributed on an | ||
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY | ||
* KIND, either express or implied. See the License for the | ||
* specific language governing permissions and limitations | ||
* under the License. | ||
*/ | ||
'use strict'; | ||
/* eslint-env node */ | ||
/* global inject */ | ||
define(['angular-mocks', 'portal'], function() { | ||
describe('mainService', function() { | ||
var service; | ||
|
||
var loginSilentUrl; | ||
var httpBackend; | ||
|
||
beforeEach(function() { | ||
module('portal'); | ||
}); | ||
|
||
beforeEach(inject(function( | ||
_mainService_, _$httpBackend_, | ||
SERVICE_LOC, MESSAGES, APP_FLAGS | ||
) { | ||
service = _mainService_; | ||
httpBackend = _$httpBackend_; | ||
loginSilentUrl = APP_FLAGS.loginOnLoad; | ||
|
||
if (loginSilentUrl) { | ||
httpBackend.whenGET(loginSilentUrl) | ||
.respond({'status': 'success', 'username': 'admin'}); | ||
} | ||
|
||
httpBackend.whenGET(SERVICE_LOC.sessionInfo).respond(); | ||
})); | ||
|
||
it('should set title to include page, app, and portal name when ' | ||
+ 'all of these are present and non-redundant.', function() { | ||
// setup | ||
var pageTitle = 'Timesheets'; | ||
var appTitle = 'STAR Time Entry'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle) | ||
.toEqual('Timesheets | STAR Time Entry | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to omit app name when redundant with portal name', | ||
function() { | ||
// setup | ||
var pageTitle = 'Notifications'; | ||
var appTitle = 'MyUW'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('Notifications | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to omit app name when null', function() { | ||
// setup | ||
var pageTitle = 'Notifications'; | ||
var appTitle = null; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('Notifications | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to omit app name when it is empty', function() { | ||
// setup | ||
var pageTitle = 'Notifications'; | ||
var appTitle = ''; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('Notifications | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to omit page name when it is null', | ||
function() { | ||
// setup | ||
var appTitle = 'STAR Time Entry'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(null, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('STAR Time Entry | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to omit page name when it is empty string', | ||
function() { | ||
// setup | ||
var pageTitle = ''; | ||
var appTitle = 'STAR Time Entry'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('STAR Time Entry | MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to only the portal name when the app name is ' | ||
+ 'redundant and the page name is not provided.', function() { | ||
// setup | ||
var pageTitle = ''; | ||
var appTitle = 'MyUW'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to only the portal name when redundant with both ' | ||
+ 'the app name and the page name.', function() { | ||
// setup | ||
var pageTitle = 'MyUW'; | ||
var appTitle = 'MyUW'; | ||
var portalTitle = 'MyUW'; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('MyUW'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to only the app name when portal name and page ' | ||
+ 'name are null', function() { | ||
// setup | ||
var pageTitle = null; | ||
var appTitle = 'STAR Time and Leave'; | ||
var portalTitle = null; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('STAR Time and Leave'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should set title to only the page name when portal name and app ' | ||
+ 'name are null', function() { | ||
// setup | ||
var pageTitle = 'SomePage'; | ||
var appTitle = null; | ||
var portalTitle = null; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('SomePage'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
|
||
it('should gracefully handle missing theme.', function() { | ||
// setup | ||
var pageTitle = 'SomePage'; | ||
var appTitle = 'SomeApp'; | ||
var portalTitle = null; | ||
|
||
// test | ||
var windowTitle = | ||
service.computeWindowTitle(pageTitle, appTitle, portalTitle); | ||
|
||
expect(windowTitle).toEqual('SomePage | SomeApp'); | ||
|
||
httpBackend.flush(); | ||
}); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,11 +23,9 @@ define(['angular'], function(angular) { | |
|
||
.factory('mainService', [ | ||
'$http', '$log', '$sessionStorage', | ||
'miscService', 'SERVICE_LOC', 'APP_FLAGS', '$rootScope', 'NAMES', | ||
'$document', | ||
'miscService', 'SERVICE_LOC', 'APP_FLAGS', 'NAMES', | ||
function($http, $log, $sessionStorage, | ||
miscService, SERVICE_LOC, APP_FLAGS, $rootScope, NAMES, | ||
$document) { | ||
miscService, SERVICE_LOC, APP_FLAGS, NAMES) { | ||
var prom = $http.get(SERVICE_LOC.sessionInfo, {cache: true}); | ||
var userPromise; | ||
|
||
|
@@ -74,26 +72,73 @@ define(['angular'], function(angular) { | |
} | ||
|
||
/** | ||
* set the frame title using theme | ||
* Compute the window title | ||
* | ||
* Returns | ||
* page-title | app-title | portal-title (in an application), or | ||
* page-title | portal-title (if the app has same name as the portal), or | ||
* app-title | portal-title (if page name unknown or redundant) | ||
* portal-title (if page name unknown or redundant and app name redundant) | ||
* | ||
* Examples: | ||
* "Timesheets | STAR Time Entry | MyUW" , | ||
* for the Timesheets page in an app named "STAR Time Entry" in | ||
* a portal named "MyUW", or | ||
* | ||
* "Notifications | MyUW", | ||
* for the notifications page in an app named "MyUW" in | ||
* a portal named "MyUW". | ||
* | ||
* "STAR Time Entry | MyUW" | ||
* in an app named "STAR Time Entry" in a portal named "MyUW" | ||
* when the page name is unspecified. | ||
* | ||
* "MyUW" | ||
* in an app named "MyUW" in a portal named "MyUW" | ||
* when the page name is unspecified. | ||
*/ | ||
function setTitle() { | ||
var frameTitle = ''; | ||
if ($rootScope.portal && $rootScope.portal.theme) { | ||
frameTitle = $rootScope.portal.theme.title; | ||
if (frameTitle !== NAMES.title && !APP_FLAGS.isWeb) { | ||
frameTitle = ' | ' + frameTitle; | ||
} else { | ||
// since frame title equals the title in NAMES lets not duplicate it | ||
frameTitle = ''; | ||
function computeWindowTitle(pageTitle, appTitle, portalTitle) { | ||
var windowTitle = ''; // we finally set the window title to this. | ||
|
||
// assemble the window title from the gathered pieces, | ||
// starting from the end of the window title and working back to the | ||
// beginning. | ||
|
||
// if the portal has a name, end the window title with that | ||
// (if the portal lacks a name, portalTitle is still empty string) | ||
|
||
if (portalTitle) { | ||
windowTitle = portalTitle; | ||
} | ||
|
||
// if the app name differs from the portal, prepend it. | ||
// if it's the same name, omit it to avoid silly redundancy like | ||
// "MyUW | MyUW" | ||
if (appTitle && appTitle !== portalTitle) { | ||
// if the windowTitle already has content, first add a separator | ||
if (windowTitle) { | ||
windowTitle = ' | ' + windowTitle; | ||
} | ||
windowTitle = appTitle + windowTitle; | ||
} | ||
$document[0].title = NAMES.title + frameTitle; | ||
|
||
// if there's a page name not redundant with the app name, prepend it. | ||
if (pageTitle && pageTitle !== '' && pageTitle !== appTitle) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
// if the windowTitle already has content, first add a separator | ||
if (windowTitle !== '') { | ||
windowTitle = ' | ' + windowTitle; | ||
} | ||
windowTitle = pageTitle + windowTitle; | ||
} | ||
|
||
// finally, return the built up windowTitle | ||
return windowTitle; | ||
} | ||
|
||
return { | ||
getUser: getUser, | ||
isGuest: isGuest, | ||
setTitle: setTitle, | ||
computeWindowTitle: computeWindowTitle, | ||
}; | ||
}]); | ||
}); |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🎨 jsDoc syntax for optional is
reference: http://usejsdoc.org/tags-param.html#optional-parameters-and-default-values