Skip to content

Improve T&C (and all) mobile rendering (CFM-501) #447

Open
wants to merge 13 commits into
base: develop
Choose a base branch
from
+960 −555
Open

Improve T&C (and all) mobile rendering (CFM-501) #447

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
98f489a
Improve UI/UX for Terms and Conditions Review (CFM-501)
arlen Jun 15, 2026
b8d117d
Move common code and language strings from TermsAgreer plugin to core…
arlen Jun 18, 2026
76436fb
Further cleanup to Terms and Conditions (CFM-501)
arlen Jun 26, 2026
d247605
Fix preview link to TandC Mostly Static Page (CFM-501)
arlen Jun 26, 2026
10b8ee5
Add JavaScript nonce to TandC script tag (CFM-501)
arlen Jun 28, 2026
628ec8e
Implement ajax UI for CO-level Terms and Conditions agreements (CFM-501)
arlen Jul 15, 2026
f8fe46e
Simplify TermsAndConditionsController::review() now that ajax is used…
arlen Aug 10, 2026
37f7d66
Move ajax request check from maybeEnforceTAndCs() to beforeFilter() (…
arlen Aug 20, 2026
15e99bd
Add inline-alert feature (CFM-501)
arlen Aug 27, 2026
faffd00
Improve T&C (and all) mobile rendering (CFM-501)
arlen Aug 30, 2026
2c74fc9
Ensure elements under #main can overflow when necessary (CFM-501)
arlen Sep 3, 2026
2fd4840
Ensure hover state underlines ignore icons (CFM-501)
arlen Sep 4, 2026
748c0c8
Move TAndCEnrollmentModeEnum to core TAndCAgreementModeEnum (CFM-510)
arlen Sep 16, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions app/config/routes.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ function (RouteBuilder $builder) {
['controller' => 'ApiV2', 'action' => 'generateApiKey', 'model' => 'api_users'])
->setPass(['id'])
->setPatterns(['id' => '[0-9]+']);
$builder->post(
'/terms_and_conditions/record/{id}',
['controller' => 'ApiV2', 'action' => 'recordTAndC', 'model' => 'terms_and_conditions'])
->setPass(['id'])
->setPatterns(['id' => '[0-9]+']);
$builder->get(
'/people/pick',
['controller' => 'ApiV2', 'action' => 'pick', 'model' => 'people']);
Expand Down
23 changes: 16 additions & 7 deletions app/plugins/CoreEnroller/templates/element/emailVerifiers/list.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,23 @@
?>

<tr>
<td><?= $addr ?></td>
<td>
<?php if($verified): ?>
<?= __d('result', 'verified') ?>
<?php else: ?>
<span class="mr-1 badge bg-warning unverified"><?= __d('field', 'unverified')?></span>
<?= $button; ?>
<?php endif; ?>
<div class="cell-first-content">
<?= $addr ?>
</div>
</td>
<td>
<div class="cell-label">
<?= __d('field', 'status') ?>
</div>
<div class="cell-content">
<?php if($verified): ?>
<?= __d('result', 'verified') ?>
<?php else: ?>
<span class="mr-1 badge bg-warning unverified"><?= __d('field', 'unverified')?></span>
<?= $button; ?>
<?php endif; ?>
</div>
</td>
</tr>
<?php endforeach; ?>
Expand Down
31 changes: 19 additions & 12 deletions app/plugins/OrcidSource/templates/element/preview.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,18 +33,25 @@
?>

<table id="orcid-source-preview-table" class="index-table list-mode">
<thead>
<tr>
<th><?= __d('field', 'identifier') ?></th>
<th><?= __d('field', 'value') ?></th>
</tr>
</thead>
<thead>
<tr>
<th><?= __d('field', 'identifier') ?></th>
<th><?= __d('field', 'value') ?></th>
</tr>
</thead>

<tbody>
<tr>
<td><?= __d('orcid_source', 'information.orcid_source.identifier') ?></td>
<td><?= $vv_orcid ?></td>
</tr>
</tbody>
<tbody>
<tr>
<td>
<div class="cell-first-content">
<?= __d('orcid_source', 'information.orcid_source.identifier') ?>
</div>
</td>
<td>
<div class="cell-label"><?= __d('field', 'value') ?></div>
<div class="cell-content"><?= $vv_orcid ?></div>
</td>
</tr>
</tbody>
</table>

12 changes: 0 additions & 12 deletions app/plugins/TermsAgreer/resources/locales/en_US/terms_agreer.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,21 +37,9 @@ msgstr "Ignore"
msgid "enumeration.TAndCEnrollmentModeEnum.IC"
msgstr "Implied Consent"

msgid "error.TAndCAgreement.missing"
msgstr "Did not receive agreement for \"{0}\" (T&C {1})"

msgid "field.AgreementCollectors.t_and_c_mode"
msgstr "Terms and Conditions Mode"

msgid "information.AgreementCollectors.external"
msgstr "These Terms and Conditions will be loaded in an external browser window. After review, you must return to this window and click \"Agree\" to continue."

msgid "information.AgreementCollectors.review"
msgstr "You must review and agree to these Terms and Conditions before continuing."

msgid "information.AgreementCollectors.review.tc"
msgstr "Review Terms & Conditions"

msgid "result.AgreementCollectors.ignored"
msgstr "Terms and Conditions collection disabled"

Expand Down
6 changes: 3 additions & 3 deletions app/plugins/TermsAgreer/src/Controller/AgreementCollectorsController.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,8 @@

use App\Controller\StandardEnrollerController;
use App\Lib\Enum\SuspendableStatusEnum;
use App\Lib\Enum\TAndCAgreementModeEnum;
use Cake\ORM\TableRegistry;
use TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum;

class AgreementCollectorsController extends StandardEnrollerController {
protected array $paginate = [
Expand All @@ -58,7 +58,7 @@ public function dispatch(string $id) {

$cfg = $this->AgreementCollectors->get($id);

if($cfg->t_and_c_mode == TAndCEnrollmentModeEnum::Ignore) {
if($cfg->t_and_c_mode == TAndCAgreementModeEnum::Ignore) {
// If the Plugin is set to Ignore, we simply skip this step and move on.

return $this->finishStep(
Expand Down Expand Up @@ -119,7 +119,7 @@ public function dispatch(string $id) {
if(!isset($data[$key]) || $data[$key] != "1") {
$ok = false;

$this->Flash->error(__d('terms_agreer','error.TAndCAgreement.missing', [$tc->description, $tc->id]));
$this->Flash->error(__d('error','TermsAndConditions.agreement.missing', [$tc->description, $tc->id]));
}
}

Expand Down
170 changes: 3 additions & 167 deletions app/plugins/TermsAgreer/templates/AgreementCollectors/dispatch.inc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
<?php
/**
* COmanage Registry T&C Agreement Colectors Petition Fields
* COmanage Registry T&C Agreement Collectors Petition Fields
*
* Portions licensed to the University Corporation for Advanced Internet
* Development, Inc. ("UCAID") under one or more contributor license agreements.
Expand All @@ -25,169 +25,5 @@
* @license Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
*/

declare(strict_types = 1);

use \TermsAgreer\Lib\Enum\TAndCEnrollmentModeEnum;

print $this->element('flash', []);

// Make the Form fields editable
$this->Field->enableFormEditMode();
?>

<p><?= __d('terms_agreer', 'information.AgreementCollectors.review') ?></p>

<?php

// Begin the form
print $this->Form->create(null, [
'id' => 'agreement-form',
'type' => 'post'
]);


?>

<table>
<tr>
<th><?= __d('controller','TermsAndConditions',[99]) ?></th>
<th class="center"><?= __d('operation','review') ?></th>
<th><?= __d('operation','agreement') ?></th>
</tr>

<?php foreach($vv_tandc as $tc): ?>
<tr class="<?= !empty($tc->url) ? 'tc-url' : 'tc-msp' ?>">
<td>
<?= $tc['description'] ?>
</td>
<td class="center">
<button
data-tc-id="tc<?= $tc['id'] ?>"
class="btn btn-primary btn-sm tc-review-button"
type="button">
<?= __d('operation','review') ?>
</button>
<?php if(!empty($tc->url)): // We have a URL based T&C ?>
<button
data-tc-id="tc<?= $tc['id'] ?>"
id="tc<?= $tc['id'] ?>-window-launcher"
class="tc-window-launcher invisible"
data-tc-url="<?= $tc->url ?>"
type="button">
<?= __d('operation','review') . ' tc' . $tc['id'] ?>
</button>
<?php else: // We have a Mostly Static Page based T&C ?>
<?= $this->element('TermsAgreer.agreeDialog', ['vv_tc' => $tc]); ?>
<?php endif; ?>
</td>
<td>
<div class="form-check tc-agree">
<?=
$this->Form->checkbox(
'tc'.$tc['id'],
['id' => 'tc'.$tc['id'], 'class' => 'form-check-input tc-agree-checkbox']
) .
$this->Form->label(
'tc'.$tc['id'],
__d('operation','agree'),
['class' => 'form-check-label']
)
?>
</div>
</td>
</tr>
<?php endforeach; ?>

</table>


<script nonce="<?= $vv_js_nonce ?>">
// Iterate over our T&Cs to see if they're all checked.
function checkAgreements(mode) {
let allAgreed = true;
$('.tc-agree-checkbox').each(function() {
if(!$(this).prop('checked')) {
allAgreed = false;
if(mode == 'EC') {
// We're in Explicit Consent mode, so disable
// the current checkbox; its T&C must be reviewed.
$(this).prop('disabled', true);
}
}
});

// Enable / disable the "Continue" button.
if(allAgreed) {
$('.submit input[type="submit"]').prop('disabled', false);
} else {
$('.submit input[type="submit"]').prop('disabled', true);
}
}

// On page load: check agreement states and attach event handlers.
$(function() {
// Explicit Consent mode ("EC") will disable "I Agree" checkboxes until review.
const tcmode = '<?= $vv_tandc_mode ?>';

// Run on first load.
checkAgreements(tcmode);

// Handle "Review" button for URL-based terms and conditions.
$('tr.tc-url button.tc-review-button').click(function(e) {
e.preventDefault();
jsConfirmGeneric(
'<?= __d('terms_agreer', 'information.AgreementCollectors.external') ?>',
'',
$(this).attr('data-tc-id')+'-window-launcher',
'<?= __d('operation','continue') ?>',
'<?= __d('operation','cancel') ?>',
'<?= __d('terms_agreer','information.AgreementCollectors.review.tc') ?>'
);
});

// Launch the external window for URL-based terms and conditions
$('tr.tc-url button.tc-window-launcher').click(function(e) {
e.preventDefault();

// Calculate window size based on screen size so that we
// can handle mobile and to center the window.
const winWidth = window.outerWidth / 1.5;
const winHeight = window.outerHeight / 1.5;
const left = window.screenLeft + ((window.outerWidth - winWidth) / 2);
const top = window.screenTop + ((window.outerHeight - winHeight) / 2);

// Launch the T&C window with the external URL
let tandcExternalWindow = window.open(
$(this).attr('data-tc-url'),
'tandcExternalWindow',
'width=' + winWidth + ',height=' + winHeight + ',left=' + left + ',top=' + top +
',resizable=yes,scrollbars=yes,toolbar=no,menubar=no,location=no,status=no'
);

// Enable the T&C checkbox
$('#'+$(this).attr('data-tc-id')).prop('disabled', false);
});

// Handle "Review" button for Mostly Static Page-based terms and conditions.
// Clicking this will reveal the T&C agree dialog.
$('tr.tc-msp button.tc-review-button').click(function(e) {
e.preventDefault();
$('#'+$(this).attr('data-tc-id')+'-agree-dialog').modal('show');
});

// Handle "Agree" button click from MSP Dialog
$('.tc-agree-button').click(function(e) {
// Enable and check the T&C checkbox
$('#'+$(this).attr('data-tc-id'))
.prop('checked', true)
.prop('disabled', false)
.trigger('change');
});

// Check agreements when a checkbox is clicked.
// If all are checked, we'll enable "Continue".
$('.tc-agree-checkbox').change(function() {
checkAgreements(tcmode);
});
});
</script>
// This dispatch uses the same user interface as the core TermsAndConditions/tandcs.inc template.
include ROOT . DS . 'templates' . DS . 'TermsAndConditions' . DS . 'tandcs.inc';
6 changes: 6 additions & 0 deletions app/resources/locales/en_US/error.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -451,6 +451,12 @@ msgstr "No outgoing SMTP Server configuration found"
msgid "TermsAndConditions.document.one"
msgstr "Exactly one of Mostly Static Page or URL must be specified"

msgid "TermsAndConditions.agreement.missing"
msgstr "Did not receive agreement for \"{0}\" (T&C {1})"

msgid "TermsAndConditions.agreement.failed"
msgstr "Failed to record agreement. Please try again or check with your administrator."

msgid "TermsAndConditions.revoke.none"
msgstr "No Agreements available to revoke"

Expand Down
11 changes: 10 additions & 1 deletion app/resources/locales/en_US/information.po
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -235,7 +235,16 @@ msgid "table.list"
msgstr "{0} List"

msgid "TermsAndConditions.desc.old"
msgstr "{0} (Agreement is to an older definition)"
msgstr "Agreement is to an older definition"

msgid "tandc.review"
msgstr "You must review and agree to these Terms and Conditions before continuing."

msgid "tandc.review.dialog.title"
msgstr "Review Terms & Conditions"

msgid "tandc.review.external"
msgstr "These Terms and Conditions will be loaded in an external browser window. After review, you must return to this window and click \"Agree\" to continue."

msgid "ug.tasks.post"
msgstr "Executing post-database tasks for version {0}"
Expand Down
28 changes: 10 additions & 18 deletions app/src/Controller/ApiV2Controller.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,21 +131,6 @@ public function add() {
$this->render('/Standard/api/v2/json/add-edit');
}

/**
* beforeFilter callback.
*
* @param \Cake\Event\EventInterface $event Event.
* @return \Cake\Http\Response|null|void
*/
public function beforeFilter(\Cake\Event\EventInterface $event)
{
parent::beforeFilter($event);

if ($this->request->is('ajax') && $this->request->is(['post', 'put'])) {
$this->FormProtection->setConfig('validate', false);
}
}

/**
* Callback run prior to the request rendering.
*
Expand Down Expand Up @@ -446,20 +431,27 @@ public function recordTAndC(string $id) {
}

$TAndCAgreements = TableRegistry::getTableLocator()->get('TAndCAgreements');
$TAndCs = TableRegistry::getTableLocator()->get('TermsAndConditions');

$agreement = $TAndCAgreements->record(
termsAndConditionsId: (int)$id,
personId: $json['personId'],
actorPersonId: $json['actorPersonId'],
personId: (int)$json['personId'],
actorPersonId: (int)$json['actorPersonId'],
Comment on lines -452 to +439
Copy link
Contributor

@benno benno Sep 15, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realize this is existing code, but what's to stop somebody from sending a random person ID as either the subject or the actor? The original intent of the /record API was for API Users, not AJAX requests, and API Users are already trusted to make calls on behalf of other users.

We probably need to split recordTAndC into two: one for the "regular" API and one for the AJAX API. The second call can set the Actor Person ID based on the currently authenticated user. Person ID would either (1) be the currently authenticated user, or (2) if the user is an Admin, allow Person ID to be asserted if it is any Person the Actor can manage.

Copy link
Contributor Author

@arlen arlen Sep 16, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with this concern. Can we break this issue out into a different ticket independent of this PR since this one doesn't change the existing code (other than to ensure the (int) type is properly cast)?

identifier: $json['identifier']
);

$TAndCAgreements->People->requestProvisioning(
id: $json['personId'],
id: (int)$json['personId'],
context: ProvisioningContextEnum::Automatic
);

$this->set('vv_results', ['id' => $agreement->id]);

// Only this action uses a Flash message. If more restful actions need it in the future,
// move the loadComponent('Flash') out of the !restful test in AppController.
$this->loadComponent('Flash');
$tandc = $TAndCs->get($agreement->terms_and_conditions_id);
$this->Flash->success(__d('result', 'TermsAndConditions.agreed', [$tandc->description]));

// Let the view render
$this->viewBuilder()->setLayout('rest');
Expand Down
Loading