Skip to content

Commit

Permalink
Ensure selected idp list works right (#1074)
Browse files Browse the repository at this point in the history
Prior to this change deleting a selected idp was not possible.  While it was visually removed, returning to the wave later showed it was still there.

This change ensures the behaviour is as expected:
- hitting delete button truly deletes it
- returning to the wave without a hard refresh will show the correct list of already selected idps, even if returning via the backbutton

[Issue discovered as part of fixing tickets for the latest feedback round](https://wiki.surfnet.nl/display/coininfra/Bevindingen+nav+tests+door+supportteam)
  • Loading branch information
Koen Cornelis authored Feb 1, 2021
1 parent 6abcb30 commit 492e156
Show file tree
Hide file tree
Showing 16 changed files with 131 additions and 76 deletions.
13 changes: 3 additions & 10 deletions theme/base/javascripts/handlers.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,8 @@ import {switchConsentSection} from './consent/switchConsentSection';
import {addClickHandlerOnce} from './utility/addClickHandlerOnce';
import {
backButtonSelector,
configurationId,
nokButtonSelector,
nokSectionSelector,
selectedIdpsSectionSelector,
tooltipsAndModalLabels,
} from './selectors';
import {handlePreviousSelectionVisible} from './wayf/handlePreviousSelectionVisible';
Expand Down Expand Up @@ -53,19 +51,14 @@ export const backButtonHandler = (nokSection) => {
* WAYF HANDLERS
* ***/
export const wayfCallbackAfterLoad = () => {
// Initialize variables
const selectedIdps = document.querySelector(selectedIdpsSectionSelector);
const configuration = JSON.parse(document.getElementById(configurationId).innerHTML);
const previouslySelectedIdps = configuration.previousSelectionList;

// Initialize behaviour
handlePreviousSelectionVisible(selectedIdps, previouslySelectedIdps);
wayfKeyboardBehaviour(previouslySelectedIdps);
handlePreviousSelectionVisible();
wayfKeyboardBehaviour();
mouseBehaviour();
searchBehaviour();
};
export const idpSubmitHandler = (e) => {
submitForm(e, true);
submitForm(e);
};
export const cancelButtonClickHandler = (parentSection, noAccess) => cancelButtonClickHandlerCreator(parentSection, noAccess);
export const requestButtonHandler = () => { toggleFormFieldsAndButton(); };
2 changes: 2 additions & 0 deletions theme/base/javascripts/selectors.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,9 @@ export const wayfPageSelector = 'main.wayf';
export const configurationId = 'wayf-configuration';
export const selectedIdpsSectionSelector = '.wayf__previousSelection';
export const selectedIdpsListSelector = '.wayf__previousSelection .wayf__idpList';
export const selectedIdpsLiSelector = `${selectedIdpsListSelector} > li`;
export const previousSelectionFirstIdp = '.wayf__previousSelection li:first-of-type .wayf__idp';
export const deleteButtonTemplateId = 'deleteButton';
export const addAccountButtonClass = 'previousSelection__addAccount';
export const addAccountButtonSelector = `.${addAccountButtonClass}`;
export const idpClass = 'wayf__idp';
Expand Down
24 changes: 11 additions & 13 deletions theme/base/javascripts/wayf/deleteDisable/addSelectedIdp.js
Original file line number Diff line number Diff line change
@@ -1,34 +1,32 @@
import {savePreviousSelection} from './savePreviousSelection';
import {configurationId} from '../../selectors';
import * as Cookies from 'js-cookie';

/**
* Add the selected idp to the list of previouslyselected idps and save it to the cookie.
*
* @param isPreviouslySelectedList
* @param element
*/
export const addSelectedIdp = (isPreviouslySelectedList, element) => {
const configuration = JSON.parse(document.getElementById(configurationId).innerHTML);
const cookieName = configuration.previousSelectionCookieName;
let previouslySelectedIdps = configuration.previousSelectionList;
export const addSelectedIdp = (element) => {
const cookieName = JSON.parse(document.getElementById(configurationId).innerHTML).previousSelectionCookieName;
const entityId = element.getAttribute('data-entityid');
const count = Number(element.getAttribute('data-count'));
let alreadyInCookie = false;
let cookie = Cookies.get(cookieName) || [];

if (isPreviouslySelectedList) {
previouslySelectedIdps.forEach(idp => {
if (idp.entityId === entityId) {
if (cookie.length) {
cookie = JSON.parse(cookie);
cookie.forEach(idp => {
if (idp.idp === entityId) {
idp.count += 1;
savePreviousSelection(previouslySelectedIdps, cookieName);
savePreviousSelection(cookie, cookieName);
alreadyInCookie = true;
return;
}
});
}

if (!alreadyInCookie) {
previouslySelectedIdps = [...previouslySelectedIdps, { entityId: entityId, count: (count + 1) }];
cookie = [...cookie, { idp: entityId, count: 1 }];
savePreviousSelection(cookie, cookieName);
}

savePreviousSelection(previouslySelectedIdps, cookieName);
};
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,9 @@ import {idpDeleteDisabledSelector} from '../../selectors';

/**
* Ensure all delete buttons can actually delete.
*
* @param previouslySelectedIdps
*/
export const attachDeleteHandlers = (previouslySelectedIdps) => {
export const attachDeleteHandlers = () => {
const buttons = document.querySelectorAll(idpDeleteDisabledSelector);

buttons.forEach(button => button.addEventListener('click', (e) => handleDeleteDisable(e, previouslySelectedIdps)));
buttons.forEach(button => button.addEventListener('click', (e) => handleDeleteDisable(e)));
};
21 changes: 14 additions & 7 deletions theme/base/javascripts/wayf/deleteDisable/deleteIdp.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,26 @@
import {savePreviousSelection} from './savePreviousSelection';
import {configurationId, idpSelector} from '../../selectors';
import * as Cookies from 'js-cookie';
import {getData} from '../../utility/getData';

/**
* Delete an idp from the previous selection in both html & the previous selection list
*
* @param element HTMLElement the deleted idp
* @param previousSelection Array the list of previously selected idps
*/
export const deleteIdp = (element, previousSelection) => {
const configuration = JSON.parse(document.getElementById(configurationId).innerHTML);
const cookieName = configuration.previousSelectionCookieName;
export const deleteIdp = (element) => {
const cookieName = JSON.parse(document.getElementById(configurationId).innerHTML).previousSelectionCookieName;
const idp = element.closest(idpSelector);
const index = Number(idp.getAttribute('data-index'));
previousSelection.splice((index - 1), 1);
savePreviousSelection(previousSelection, cookieName);
const id = getData(idp, 'entityid');
const cookie = JSON.parse(Cookies.get(cookieName));

cookie.forEach((idp, index) => {
if (idp.idp === id) {
cookie.splice((index - 1), 1);
}
});

savePreviousSelection(cookie, cookieName);

// Remove deleted item from html
idp.closest('li').remove();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,10 @@ import {setCookie} from '../../utility/setCookie';
* @param cookieName string
*/
export const savePreviousSelection = (previousSelection, cookieName) => {
const simplifiedPreviousSelection = previousSelection.map((idp) => {
return {
'idp': idp.entityId,
'count': idp.count
};
});
if (previousSelection.length === 0) {
setCookie(previousSelection, cookieName, -1, '/', true);
return;
}

setCookie(simplifiedPreviousSelection, cookieName, 365, '/', true);
setCookie(previousSelection, cookieName, 365, '/', true);
};
9 changes: 4 additions & 5 deletions theme/base/javascripts/wayf/handleDeleteDisable.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {hasSelectedIdps} from './utility/hasSelectedIdps';
import {hasSelectedIdpsInList} from './utility/hasSelectedIdpsInList';
import {handleNoneLeft} from './deleteDisable/handleNoneLeft';
import {deleteIdp} from './deleteDisable/deleteIdp';
import {reindexIdpArray} from './utility/reindexIdpArray';
Expand All @@ -13,9 +13,8 @@ import {idpDeleteDisabledSelector, idpSelector} from '../selectors';
* Handle what happens if a user clicks on either the delete button, or the disabled button in an Idp.
*
* @param e
* @param previouslySelectedIdps
*/
export const handleDeleteDisable = (e, previouslySelectedIdps) => {
export const handleDeleteDisable = (e) => {
e.preventDefault();
e.stopPropagation();
let element = e.target;
Expand All @@ -32,7 +31,7 @@ export const handleDeleteDisable = (e, previouslySelectedIdps) => {
}

// Remove item from previous selection & html
deleteIdp(element, previouslySelectedIdps);
deleteIdp(element);

// Reindex & SortRemaining idps by title
const idpArray = sortRemaining();
Expand All @@ -42,7 +41,7 @@ export const handleDeleteDisable = (e, previouslySelectedIdps) => {
}

// If no items are left: do what's needed.
if (!hasSelectedIdps()) {
if (!hasSelectedIdpsInList()) {
handleNoneLeft();
return;
}
Expand Down
6 changes: 3 additions & 3 deletions theme/base/javascripts/wayf/handleEnter.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import {idpSubmitHandler} from '../handlers';
/**
* Behaviour expected to happen after a user presses the enter button.
*/
export const handleEnter = (e, previouslySelectedIdps) => {
export const handleEnter = (e) => {
const classList = e.target.classList;

classList.forEach(className => {
Expand All @@ -32,7 +32,7 @@ export const handleEnter = (e, previouslySelectedIdps) => {
case idpDeleteDisabledClass:
case idpDeleteClass:
case idpDisabledClass:
handleDeleteDisable(e, previouslySelectedIdps);
handleDeleteDisable(e);
break;
// select an idp
case idpClass:
Expand All @@ -43,7 +43,7 @@ export const handleEnter = (e, previouslySelectedIdps) => {
case idpLogoClass:
idpSubmitHandler(e); break;
case searchFieldClass:
selectFirstIdPAndSubmitForm(previouslySelectedIdps); break;
selectFirstIdPAndSubmitForm(); break;
case defaultIdpClass:
handleIdpBanner(e); break;
}
Expand Down
18 changes: 9 additions & 9 deletions theme/base/javascripts/wayf/handlePreviousSelectionVisible.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,22 @@
import {hasSelectedIdps} from './utility/hasSelectedIdps';
import {hasSelectedIdpsInList} from './utility/hasSelectedIdpsInList';
import {attachDeleteHandlers} from './deleteDisable/attachDeleteHandlers';
import {switchIdpSection} from './utility/switchIdpSection';
import {focusOn} from "../utility/focusOn";
import {addAccountButtonSelector, previousSelectionFirstIdp, selectedIdpsListSelector} from '../selectors';
import {addClickHandlerOnce} from '../utility/addClickHandlerOnce';
import {idpSubmitHandler} from '../handlers';
import {matchPreviouslySelectedWithCookie} from './matchPreviouslySelectedWithCookie';

/**
* Check if user has any previous selected Idps.
* If so: show those & attach mouse handlers.
* Else: do nothing as the remaining idps are shown by default.
*
* @param selectedIdps HTMLElement the list of user-selected idps
* @param previouslySelectedIdps Array the list of previously selected idps
*/
export const handlePreviousSelectionVisible = (selectedIdps, previouslySelectedIdps) => {
if (hasSelectedIdps()) {
mouseHandlersHiddenIdps(previouslySelectedIdps);
export const handlePreviousSelectionVisible = () => {
matchPreviouslySelectedWithCookie();

if (hasSelectedIdpsInList()) {
mouseHandlersHiddenIdps();
// put focus on the first IDP, so you can just hit enter & go
focusOn(previousSelectionFirstIdp);
}
Expand All @@ -25,12 +25,12 @@ export const handlePreviousSelectionVisible = (selectedIdps, previouslySelectedI
/**
* Mouse handlers for the previous selection. As it's initially hidden we should not add them on load, but on show of that section.
*/
const mouseHandlersHiddenIdps = (previouslySelectedIdps) => {
const mouseHandlersHiddenIdps = () => {
// Show remaining idp section when hitting the add account button
addClickHandlerOnce(addAccountButtonSelector, switchIdpSection);

// Handle clicking the "garbage can" after hitting edit
attachDeleteHandlers(previouslySelectedIdps);
attachDeleteHandlers();

// Attach event listener to previous selection idps-list
addClickHandlerOnce(selectedIdpsListSelector, idpSubmitHandler);
Expand Down
4 changes: 2 additions & 2 deletions theme/base/javascripts/wayf/keyboardBehaviour.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,14 @@ import {handleEnter} from './handleEnter';
* All handlers for the expected keyboard behaviour.
* Grouped by key.
*/
export const keyboardBehaviour = (previouslySelectedIdps) => {
export const keyboardBehaviour = () => {
const ENTER = 13;
const ARROW_UP = 38;
const ARROW_DOWN = 40;

document.querySelector('body').addEventListener('keydown', function(e) {
if (e.keyCode === ENTER) {
handleEnter(e, previouslySelectedIdps);
handleEnter(e);
return;
}

Expand Down
65 changes: 65 additions & 0 deletions theme/base/javascripts/wayf/matchPreviouslySelectedWithCookie.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
import {configurationId, deleteButtonTemplateId, idpContentClass, idpDeleteSelector, idpDeleteDisabledSelector, remainingIdpSelector, selectedIdpsListSelector, selectedIdpsSelector, selectedIdpsLiSelector} from '../selectors';
import * as Cookies from 'js-cookie';
import {sortIdpList} from './utility/sortIdpList';
import {getData} from '../utility/getData';

export const matchPreviouslySelectedWithCookie = () => {
const cookieName = JSON.parse(document.getElementById(configurationId).innerHTML).previousSelectionCookieName;
const selectedIdps = document.querySelectorAll(selectedIdpsSelector);
const selectedIdpList = document.querySelector(selectedIdpsListSelector);
let cookie = Cookies.get(cookieName);
let listMustBeSorted = false;

if (cookie) {
if (selectedIdps) {
for (const idp of selectedIdps) {
const id = getData(idp, 'entityid');

// check if idp is in cookie, if not remove it from the list
if (cookie && cookie.indexOf(id) === -1) {
idp.parentElement.remove();
listMustBeSorted = true;
}
}
}

cookie = JSON.parse(cookie);

// check if each idp in the cookie is in the list, if not add it
cookie.forEach(idp => {
const id = `[data-entityid="${idp.idp}"]`;
const inPreviouslySelected = document.querySelector(`${selectedIdpsSelector}${id}`);

if (!inPreviouslySelected) {
const deleteButtonTemplate = document.getElementById(deleteButtonTemplateId);
const clone = document.querySelector(`${remainingIdpSelector}${id}`).parentElement.cloneNode(true);
const hasDeleteDisabledButton = clone.querySelector(idpDeleteDisabledSelector);

// disabled idps
if (hasDeleteDisabledButton) {
clone.querySelector(idpDeleteDisabledSelector).appendChild(deleteButtonTemplate.querySelector(idpDeleteSelector).cloneNode(true));
}

// non-disabled idps
if (!hasDeleteDisabledButton) {
clone.querySelector(`.${idpContentClass}`).appendChild(deleteButtonTemplate.content.cloneNode(true));
}

selectedIdpList.appendChild(clone);
listMustBeSorted = true;
}

if (inPreviouslySelected) {
const count = getData(inPreviouslySelected, 'count');
if (count !== idp.count) {
inPreviouslySelected.setAttribute('data-count', idp.count);
}
}
});

if (listMustBeSorted) {
const selectedIdpLis = document.querySelectorAll(selectedIdpsLiSelector);
sortIdpList(selectedIdpLis, 'previous');
}
}
};
6 changes: 2 additions & 4 deletions theme/base/javascripts/wayf/selectFirstIdPAndSubmitForm.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,11 @@ import {firstRemainingIdpAfterSearchSelector} from '../selectors';

/**
* Selects the first visible IdP from the WAYF IdP list and submits that IdP
*
* @param previouslySelectedIdPs
*/
export const selectFirstIdPAndSubmitForm = (previouslySelectedIdPs) => {
export const selectFirstIdPAndSubmitForm = () => {
let idp = document.querySelector(firstRemainingIdpAfterSearchSelector);
// If any visible IdPs where present, choose and submit that one
if (idp) {
selectAndSubmit(idp, previouslySelectedIdPs);
selectAndSubmit(idp);
}
};
Loading

0 comments on commit 492e156

Please sign in to comment.