From dd5804d39e43f6e5ea1d99e87b3fc04eb4277b6d Mon Sep 17 00:00:00 2001
From: Arlen Johnson <arlen@sphericalcowgroup.com>
Date: Wed, 28 Dec 2022 11:35:12 -0500
Subject: [PATCH] Update flash messages to better support RTL languages
 (CFM-221) (#65)

* Update flash messages to better support RTL languages and render them in an element container so the messages can be placed under the appropriate title depending on the existence of subnavigation. (CFM-221)

* Improve code comments/documentation. (CFM-221)
---
 app/resources/locales/en_US/information.po  | 12 ------
 app/templates/Standard/add-edit-view.php    | 32 ++++++++-------
 app/templates/Standard/index.php            | 35 ++++++++--------
 app/templates/element/flash.php             | 44 +++++++++++++++++++++
 app/templates/element/flash/default.php     | 10 ++---
 app/templates/element/flash/error.php       |  8 ++--
 app/templates/element/flash/information.php | 11 +++---
 app/templates/element/flash/success.php     | 11 ++++--
 8 files changed, 100 insertions(+), 63 deletions(-)
 create mode 100644 app/templates/element/flash.php

diff --git a/app/resources/locales/en_US/information.po b/app/resources/locales/en_US/information.po
index fa0fe170c..9ae8226af 100644
--- a/app/resources/locales/en_US/information.po
+++ b/app/resources/locales/en_US/information.po
@@ -36,18 +36,6 @@ msgstr "You are not an active member in any collaboration. If your request for e
 msgid "cos.select"
 msgstr "Please select the collaboration (CO) you wish to manage."
 
-msgid "flash.default"
-msgstr "Notice: "
-
-msgid "flash.error"
-msgstr "Error: "
-
-msgid "flash.information"
-msgstr "Information: "
-
-msgid "flash.success"
-msgstr "Success: "
-
 msgid "entity.id"
 msgstr "ID: {0}"
 
diff --git a/app/templates/Standard/add-edit-view.php b/app/templates/Standard/add-edit-view.php
index cce8353c9..09020cc13 100644
--- a/app/templates/Standard/add-edit-view.php
+++ b/app/templates/Standard/add-edit-view.php
@@ -52,6 +52,15 @@
 if(!empty($vv_primary_link) && !empty($this->request->getQuery($vv_primary_link))) {
   $linkFilter = [$vv_primary_link => $this->request->getQuery($vv_primary_link)];
 }
+
+// $flashArgs pass banner messages to the flash element container
+$flashArgs = [];
+if(!empty($banners)) {
+  // XXX this doesn't work yet because we don't include fields.inc until later
+  //     either create a second file to include earlier, or use a function to emit
+  //     the fields (which would be more consistent with how Views render...)
+  $flashArgs['vv_banners'] = $banners;
+}
 ?>
 
 <?php if(!empty($subnav)): ?>
@@ -65,6 +74,10 @@
         <?php endif; ?>
       </h1>
     </div>
+    
+    <?php /* Flash Messages are placed below supertitle when subnavigation exists. */ ?>
+    <?= $this->element('flash', $flashArgs); ?>
+    
     <?= $this->element('subnavigation', $subnav); ?>
   </div>
 <?php endif; ?>
@@ -142,21 +155,10 @@
   ?>
 </div>
 
-<!-- Flash Messages and defined Info Banners -->
-<div class="alert-container" id="flash-messages">
-  <?= $this->Flash->render() ?>
-
-  <?php
-  // XXX this doesn't work yet because we don't include fields.inc until later
-  //     either create a second file to include earlier, or use a function to emit
-  //     the fields (which would be more consistent with how Views render...)
-  if(!empty($banners)) {
-    foreach($banners as $b) {
-      print $this->Alert->alert($b, 'warning');
-    }
-  }
-  ?>
-</div>
+<?php if(empty($subnav)): ?>
+  <?php /* Flash Messages are placed below the main title when there's no subnavigation. */ ?>
+  <?= $this->element('flash', $flashArgs); ?>
+<?php endif; ?>
 
 <?php
 // By default, the form will POST to the current controller
diff --git a/app/templates/Standard/index.php b/app/templates/Standard/index.php
index 3a12f0525..f13cac037 100644
--- a/app/templates/Standard/index.php
+++ b/app/templates/Standard/index.php
@@ -58,6 +58,15 @@
 if(!empty($vv_primary_link) && !empty($this->request->getQuery($vv_primary_link))) {
   $linkFilter = [$vv_primary_link => $this->request->getQuery($vv_primary_link)];
 }
+
+// $flashArgs pass banner messages to the flash element container
+$flashArgs = [];
+if(!empty($indexBanners)) {
+  $flashArgs['vv_index_banners'] = $indexBanners;
+}
+if(!empty($banners)) {
+  $flashArgs['vv_banners'] = $banners;
+}
 ?>
 
 <?php if(!empty($subnav)): ?>
@@ -71,6 +80,10 @@
         <?php endif; ?>
       </h1>
     </div>
+
+    <?php /* Flash Messages are placed below supertitle when subnavigation exists. */ ?>
+    <?= $this->element('flash', $flashArgs); ?>
+    
     <?= $this->element('subnavigation', $subnav); ?>
   </div>
 <?php endif; ?>
@@ -147,23 +160,11 @@
   <?php endif; ?>
 </div>
 
-<!-- Flash Messages and defined Info Banners -->
-<div class="alert-container" id="flash-messages">
-  <?= $this->Flash->render() ?>
-
-  <?php if(!empty($indexBanners)): ?>
-    <?php foreach($indexBanners as $b): ?>
-      <?=  $this->Alert->alert($b, 'warning') ?>
-    <?php endforeach; // $indexBanners ?>
-  <?php endif; // $indexBanners ?>
-
-  <?php if(!empty($banners)): ?>
-    <?php foreach($banners as $b): ?>
-      <?=  $this->Alert->alert($b, 'warning') ?>
-    <?php endforeach; // $banners ?>
-  <?php endif; // $banners ?>
-</div>
-
+<?php if(empty($subnav)): ?>
+  <?php /* Flash Messages are placed below the main title when there's no subnavigation. */ ?>
+  <?= $this->element('flash', $flashArgs); ?>
+<?php endif; ?>
+  
 <!-- Search block -->
 <?php if(isset($vv_searchable_attributes)): ?>
   <?= $this->element('filter'); ?>
diff --git a/app/templates/element/flash.php b/app/templates/element/flash.php
new file mode 100644
index 000000000..5c6a91f26
--- /dev/null
+++ b/app/templates/element/flash.php
@@ -0,0 +1,44 @@
+<?php
+  /*
+   * COmanage Registry Flash Message Container
+   *
+   * Portions licensed to the University Corporation for Advanced Internet
+   * Development, Inc. ("UCAID") under one or more contributor license agreements.
+   * See the NOTICE file distributed with this work for additional information
+   * regarding copyright ownership.
+   *
+   * UCAID 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:
+   *
+   * 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.
+   *
+   * @link          https://www.internet2.edu/comanage COmanage Project
+   * @package       registry
+   * @since         COmanage Registry v5.0.0
+   * @license       Apache License, Version 2.0 (http://www.apache.org/licenses/LICENSE-2.0)
+   */
+?>
+
+<!-- Flash Messages and defined Info Banners -->
+<div class="alert-container" id="flash-messages">
+  <?= $this->Flash->render() ?>
+  
+  <?php if(!empty($vv_index_banners)): ?>
+    <?php foreach($vv_index_banners as $b): ?>
+      <?=  $this->Alert->alert($b, 'warning') ?>
+    <?php endforeach; // $vv_index_banners ?>
+  <?php endif; // $vv_index_banners ?>
+  
+  <?php if(!empty($vv_banners)): ?>
+    <?php foreach($vv_banners as $b): ?>
+      <?=  $this->Alert->alert($b, 'warning') ?>
+    <?php endforeach; // $vv_banners ?>
+  <?php endif; // $vv_banners ?>
+</div>
\ No newline at end of file
diff --git a/app/templates/element/flash/default.php b/app/templates/element/flash/default.php
index 5c05e86a8..325e35513 100644
--- a/app/templates/element/flash/default.php
+++ b/app/templates/element/flash/default.php
@@ -1,15 +1,13 @@
 <?php
-  // XXX are these classes set anywhere? Are they in use?
-  $class = 'message';
-  if (!empty($params['class'])) {
-    $class .= ' ' . $params['class'];
-  }
   if (!isset($params['escape']) || $params['escape'] !== false) {
     $message = h($message);
   }
 ?>
 
 <?php if(!empty($message)): ?>
-  <?= $this->Alert->alert($message, 'warning', true, __d('information','flash.default')) ?>
+  <?php /* CFM-221: while a prefix such as "Error: " or "Success: " can be sent with the Alert, 
+    we avoid prefixes to better support LTR languages. Prefixes, if desired, should be included 
+    directly in the language strings instead. */ ?>
+  <?= $this->Alert->alert($message, 'warning', true) ?>
 <?php endif; ?>
 
diff --git a/app/templates/element/flash/error.php b/app/templates/element/flash/error.php
index 04fd0e19a..8069a15da 100644
--- a/app/templates/element/flash/error.php
+++ b/app/templates/element/flash/error.php
@@ -3,10 +3,10 @@
     $message = h($message);
   }
 ?>
-<?php /*
-<div class="message error" onclick="this.classList.add('hidden');"><?= $message ?></div>
-*/ ?>
 
 <?php if(!empty($message)): ?>
-  <?= $this->Alert->alert($message, 'danger', true, __d('information','flash.error')) ?>
+  <?php /* CFM-221: while a prefix such as "Error: " or "Success: " can be sent with the Alert, 
+    we avoid prefixes to better support LTR languages. Prefixes, if desired, should be included 
+    directly in the language strings instead. */ ?>
+  <?= $this->Alert->alert($message, 'danger', true) ?>
 <?php endif; ?>
diff --git a/app/templates/element/flash/information.php b/app/templates/element/flash/information.php
index 1df1a9641..26dc52329 100644
--- a/app/templates/element/flash/information.php
+++ b/app/templates/element/flash/information.php
@@ -1,11 +1,12 @@
 <?php
-if (!isset($params['escape']) || $params['escape'] !== false) {
-    $message = h($message);
-}
+  if (!isset($params['escape']) || $params['escape'] !== false) {
+      $message = h($message);
+  }
 ?>
 
 <?php if(!empty($message)): ?>
-  <?php /* Note: unlike Notice, Error, and Success messages, Information messages require 
-           no prefix. That is, we don't include "Information: " in front of the message. */ ?> 
+  <?php /* CFM-221: while a prefix such as "Error: " or "Success: " can be sent with the Alert, 
+    we avoid prefixes to better support LTR languages. Prefixes, if desired, should be included 
+    directly in the language strings instead. */ ?>
   <?= $this->Alert->alert($message, 'information', true) ?>
 <?php endif; ?>
\ No newline at end of file
diff --git a/app/templates/element/flash/success.php b/app/templates/element/flash/success.php
index f71124cfa..6f0671bb0 100644
--- a/app/templates/element/flash/success.php
+++ b/app/templates/element/flash/success.php
@@ -1,9 +1,12 @@
 <?php
-if (!isset($params['escape']) || $params['escape'] !== false) {
-    $message = h($message);
-}
+  if (!isset($params['escape']) || $params['escape'] !== false) {
+      $message = h($message);
+  }
 ?>
 
 <?php if(!empty($message)): ?>
-  <?= $this->Alert->alert($message, 'success', true, __d('information','flash.success')) ?>
+  <?php /* CFM-221: while a prefix such as "Error: " or "Success: " can be sent with the Alert, 
+    we avoid prefixes to better support LTR languages. Prefixes, if desired, should be included 
+    directly in the language strings instead. */ ?>
+  <?= $this->Alert->alert($message, 'success', true) ?>
 <?php endif; ?>
\ No newline at end of file