[TASK] Streamline usage and design of FlashMessages in the BE 27/46927/4
authorFrank Naegler <frank.naegler@typo3.org>
Fri, 26 Feb 2016 22:22:31 +0000 (23:22 +0100)
committerBenjamin Kott <info@bk2k.info>
Thu, 3 Mar 2016 20:12:48 +0000 (21:12 +0100)
This patch streamline the usage and design of FlashMessages in the BE.
The markup is now centralized in a method of the FlashMessage class.
In all places where FlashMessages rendered, this method is called.

Resolves: #73698
Releases: master
Change-Id: I5ef18a95ea15949e8cede71227101f2cc1ce30d3
Reviewed-on: https://review.typo3.org/46927
Reviewed-by: Benjamin Kott <info@bk2k.info>
Tested-by: Benjamin Kott <info@bk2k.info>
Reviewed-by: Timo Schmidt <timo-schmidt@gmx.net>
Tested-by: Timo Schmidt <timo-schmidt@gmx.net>
Build/Resources/Public/Less/TYPO3/_element_message.less
typo3/sysext/backend/Classes/Form/Container/InlineRecordContainer.php
typo3/sysext/backend/Resources/Private/Templates/Module.html
typo3/sysext/core/Classes/Database/QueryView.php
typo3/sysext/core/Classes/Messaging/FlashMessage.php
typo3/sysext/core/Classes/Messaging/FlashMessageQueue.php
typo3/sysext/core/Documentation/Changelog/master/Breaking-72438-RemoveDeprecatedCodeFromFlashMessage.rst
typo3/sysext/core/Documentation/Changelog/master/Breaking-73698-StreamlineLayoutOfFlashMessages.rst [new file with mode: 0644]
typo3/sysext/fluid/Classes/ViewHelpers/FlashMessagesViewHelper.php
typo3/sysext/impexp/Resources/Private/Templates/ImportExport/Import.html

index 54266a8..7478b83 100644 (file)
@@ -1,11 +1,4 @@
 //
-// Alert wrapper
-//
-ul.typo3-messages {
-       padding: 0;
-}
-
-//
 // Alert notice
 //
 .alert-notice {
@@ -82,4 +75,4 @@ ul.typo3-messages {
                        .opacity(0.95);
                }
        }
-}
\ No newline at end of file
+}
index b79b938..7a2cc66 100644 (file)
@@ -249,6 +249,8 @@ class InlineRecordContainer extends AbstractContainer
             }
             $message = $this->getLanguageService()->sL($combinationWarningMessage);
             $markup = [];
+            // @TODO: This is not a FlashMessage! The markup must be changed and special CSS
+            // @TODO: should be created, in order to prevent confusion.
             $markup[] = '<div class="alert alert-warning">';
             $markup[] = '    <div class="media">';
             $markup[] = '        <div class="media-left">';
index eaf142e..92df8a5 100644 (file)
@@ -8,26 +8,7 @@
                <f:render partial="DocHeader" arguments="{docHeader:docHeader}" />
        </f:if>
        <div class="module-body t3js-module-body">
-               <f:flashMessages as="flashMessages" queueIdentifier="{flashMessageQueueIdentifier}">
-                       <f:for each="{flashMessages}" as="flashMessage">
-                               <div class="alert {flashMessage.class}">
-                                       <div class="media">
-                                               <div class="media-left">
-                                                       <span class="fa-stack fa-lg">
-                                                               <i class="fa fa-circle fa-stack-2x"></i>
-                                                               <i class="fa fa-{flashMessage.iconName} fa-stack-1x"></i>
-                                                       </span>
-                                               </div>
-                                               <div class="media-body">
-                                                       <f:if condition="{flashMessage.title}">
-                                                               <h4 class="alert-title">{flashMessage.title}</h4>
-                                                       </f:if>
-                                                       <div class="alert-message">{flashMessage.message}</div>
-                                               </div>
-                                       </div>
-                               </div>
-                       </f:for>
-               </f:flashMessages>
+               <f:flashMessages queueIdentifier="{flashMessageQueueIdentifier}" />
                <f:format.raw>{content}</f:format.raw>
        </div>
        <f:if condition="{formTag}">
@@ -35,4 +16,4 @@
                        </form>
                </f:then>
        </f:if>
-</div>
\ No newline at end of file
+</div>
index 4ba4477..7a3a7fd 100644 (file)
@@ -357,7 +357,7 @@ class QueryView
                 }
             }
             if (!empty($flashMessage)) {
-                $msg = $this->buildFlashMessageMarkup($flashMessage);
+                $msg = $flashMessage->getMessageAsMarkup();
             }
         }
         if ($saveStoreArray) {
@@ -377,54 +377,6 @@ class QueryView
     }
 
     /**
-     * Build markup for a FlashMessage.
-     *
-     * @param FlashMessage $flashMessage
-     *
-     * @return string
-     * @internal
-     * @TODO: This method is only a temporary solution and must be cleaned up later within a fluid view
-     */
-    protected function buildFlashMessageMarkup(FlashMessage $flashMessage)
-    {
-        $classes = array(
-            FlashMessage::NOTICE => 'notice',
-            FlashMessage::INFO => 'info',
-            FlashMessage::OK => 'success',
-            FlashMessage::WARNING => 'warning',
-            FlashMessage::ERROR => 'danger'
-        );
-
-        $icons = array(
-            FlashMessage::NOTICE => 'lightbulb-o',
-            FlashMessage::INFO => 'info',
-            FlashMessage::OK => 'check',
-            FlashMessage::WARNING => 'exclamation',
-            FlashMessage::ERROR => 'times'
-        );
-
-        $title = trim($flashMessage->getTitle());
-        $output = '';
-        $output .= '<div class="alert alert-' . $classes[$flashMessage->getSeverity()] . '" style="margin-top: 20px;">';
-        $output .= '  <div class="media">';
-        $output .= '    <div class="media-left">';
-        $output .= '      <span class="fa-stack fa-lg">';
-        $output .= '        <i class="fa fa-circle fa-stack-2x"></i>';
-        $output .= '        <i class="fa fa-' . $icons[$flashMessage->getSeverity()] . ' fa-stack-1x"></i>';
-        $output .= '      </span>';
-        $output .= '    </div>';
-        $output .= '    <div class="media-body">';
-        if (!empty($title)) {
-            $output .= '      <h4 class="alert-title">' . htmlspecialchars($title) . '</h4>';
-        }
-        $output .= '      <div class="alert-message">' . htmlspecialchars($flashMessage->getMessage()) . '</div>';
-        $output .= '    </div>';
-        $output .= '  </div>';
-        $output .= '</div>';
-        return $output;
-    }
-
-    /**
      * Query marker
      *
      * @return string
@@ -525,7 +477,13 @@ class QueryView
                         . '</table>';
                 }
                 if (!$out) {
-                    $out = '<div class="alert-info">No rows selected!</div>';
+                    $flashMessage = GeneralUtility::makeInstance(
+                        FlashMessage::class,
+                        'No rows selected!',
+                        '',
+                        FlashMessage::INFO
+                    );
+                    $out = $flashMessage->getMessageAsMarkup();
                 }
                 $cPR['header'] = 'Result';
                 $cPR['content'] = $out;
index 1c25cd1..88be316 100644 (file)
@@ -104,4 +104,32 @@ class FlashMessage extends AbstractMessage
     {
         return $this->icons[$this->severity];
     }
+
+    /**
+     * Gets the message rendered as clean and secure markup
+     *
+     * @return string
+     */
+    public function getMessageAsMarkup()
+    {
+        $messageTitle = $this->getTitle();
+        $markup = [];
+        $markup[] = '<div class="alert ' . htmlspecialchars($this->getClass()) . '">';
+        $markup[] = '    <div class="media">';
+        $markup[] = '        <div class="media-left">';
+        $markup[] = '            <span class="fa-stack fa-lg">';
+        $markup[] = '                <i class="fa fa-circle fa-stack-2x"></i>';
+        $markup[] = '                <i class="fa fa-' . htmlspecialchars($this->getIconName()) . ' fa-stack-1x"></i>';
+        $markup[] = '            </span>';
+        $markup[] = '        </div>';
+        $markup[] = '        <div class="media-body">';
+        if (!empty($messageTitle)) {
+            $markup[] = '            <h4 class="alert-title">' . htmlspecialchars($messageTitle) . '</h4>';
+        }
+        $markup[] = '            <p class="alert-message">' . htmlspecialchars($this->getMessage()) . '</p>';
+        $markup[] = '        </div>';
+        $markup[] = '    </div>';
+        $markup[] = '</div>';
+        return implode('', $markup);
+    }
 }
index 65cc5c8..4c6ee4a 100644 (file)
@@ -204,16 +204,11 @@ class FlashMessageQueue extends \SplQueue
         $content = '';
         $flashMessages = $this->getAllMessagesAndFlush();
         if (!empty($flashMessages)) {
-            $content = '<ul class="typo3-messages">';
+            $content .= '<div class="typo3-messages">';
             foreach ($flashMessages as $flashMessage) {
-                $severityClass = sprintf('alert %s', $flashMessage->getClass());
-                $messageContent = htmlspecialchars($flashMessage->getMessage());
-                if ($flashMessage->getTitle() !== '') {
-                    $messageContent = sprintf('<h4>%s</h4>', htmlspecialchars($flashMessage->getTitle())) . $messageContent;
-                }
-                $content .= sprintf('<li class="%s">%s</li>', htmlspecialchars($severityClass), $messageContent);
+                $content .= $flashMessage->getMessageAsMarkup();
             }
-            $content .= '</ul>';
+            $content .= '</div>';
         }
         return $content;
     }
index ba1a94a..a2215e6 100644 (file)
@@ -57,11 +57,11 @@ This kind of information is not static, it is a temporary and volatile informati
 2) Callouts (InfoBox-ViewHelper)
 --------------------------------
 Callouts are designed to display permanent information, a very good example is the usage in the Page-Module.
-If a user opens a system folder with the page module, the callout explains: „Hey, you try to use the page module on a sys folder, please switch to the list module“.
+If a user opens a system folder with the page module, the callout explains: 'Hey, you try to use the page module on a sys folder, please switch to the list module'.
 This ViewHelper can also be used to show some help or instruction how to use a backend module.
 
 
 3) Any other information
 ------------------------
 For any other information e.g. a list of files which has changed, must be handled in the action / view of the module or plugin. This is not a use case for a FlashMessage or Callout!
-Example: It makes  sense to show a list of a hundred files within a FlashMessage or Callout, build custom markup in the view to handle this kind of message.
+Example: Display a list of a hundred files within a FlashMessage or Callout is a bad idea, build custom markup in the view to handle this kind of message.
diff --git a/typo3/sysext/core/Documentation/Changelog/master/Breaking-73698-StreamlineLayoutOfFlashMessages.rst b/typo3/sysext/core/Documentation/Changelog/master/Breaking-73698-StreamlineLayoutOfFlashMessages.rst
new file mode 100644 (file)
index 0000000..f515653
--- /dev/null
@@ -0,0 +1,64 @@
+=====================================================
+Breaking: #73698 - Streamline layout of FlashMessages
+=====================================================
+
+Description
+===========
+
+The layout and usage of FlashMessages has been streamlined in the TYPO3 backend.
+All FlashMessages in the TYPO3 backend rendered now as <div> markup and contains an icon, the message and an optional title.
+
+Example:
+
+.. code-block:: html
+
+   <div class="alert alert-danger">
+      <div class="media">
+         <div class="media-left">
+            <span class="fa-stack fa-lg">
+               <i class="fa fa-circle fa-stack-2x"></i>
+               <i class="fa fa-times fa-stack-1x"></i>
+            </span>
+         </div>
+         <div class="media-body">
+            <h4 class="alert-title">The optional title</h4>
+            <p class="alert-message">The message goes here</p>
+         </div>
+      </div>
+   </div>
+
+
+FlashMessages that are used as inline notification should be removed and replaced with custom HTML code.
+For the core we have defined output and usage for messages:
+
+1) FlashMessages
+----------------
+
+FlashMessages are designed to inform a user about success or failure of an action, which was **triggered** by the user.
+Example: If the user deletes a record, a FlashMessage informs the user about success or failure.
+This kind of information is not static, it is a temporary and volatile information and triggered by a user action.
+
+
+2) Callouts (InfoBox-ViewHelper)
+--------------------------------
+Callouts are designed to display permanent information, a very good example is the usage in the Page-Module.
+If a user opens a system folder with the page module, the callout explains: 'Hey, you try to use the page module on a sys folder, please switch to the list module'.
+This ViewHelper can also be used to show some help or instruction how to use a backend module.
+
+
+3) Any other information
+------------------------
+For any other information e.g. a list of files which has changed, must be handled in the action / view of the module or plugin. This is not a use case for a FlashMessage or Callout!
+Example: Display a list of a hundred files within a FlashMessage or Callout is a bad idea, build custom markup in the view to handle this kind of message.
+
+
+Impact
+======
+
+Extensions which use the FlashMessageViewHelper with the default rendering will now get a list of <div>-messages instead of a <ul>-list.
+
+
+Migration
+=========
+
+No migration needed, the generated output should be as expected. If the rendering is broken please consider about the correct usage of FlashMessages and read the explanation about message types above.
index 78990b6..0a9b7ed 100644 (file)
@@ -33,7 +33,7 @@ namespace TYPO3\CMS\Fluid\ViewHelpers;
  * <f:flashMessages class="specialClass" />
  * </code>
  * <output>
- * <ul class="specialClass">
+ * <div class="specialClass">
  * ...
  * </ul>
  * </output>
@@ -42,15 +42,22 @@ namespace TYPO3\CMS\Fluid\ViewHelpers;
  * <f:flashMessages />
  * </code>
  * <output>
- * <ul class="typo3-messages">
- * <li class="alert alert-ok">
- * <h4>Some Message Header</h4>
- * Some message body
- * </li>
- * <li class="alert alert-notice">
- * Some notice message without header
- * </li>
- * </ul>
+ * <div class="typo3-messages">
+ *  <div class="alert alert-info">
+ *      <div class="media">
+ *          <div class="media-left">
+ *              <span class="fa-stack fa-lg">
+ *                  <i class="fa fa-circle fa-stack-2x"></i>
+ *                  <i class="fa fa-info fa-stack-1x"></i>
+ *              </span>
+ *          </div>
+ *          <div class="media-body">
+ *              <h4 class="alert-title">Info - Title for Info message</h4>
+ *              <p class="alert-message">Message text here.</p>
+ *          </div>
+ *      </div>
+ *  </div>
+ * </div>
  * </output>
  * <code title="Output flash messages as a description list">
  * <f:flashMessages as="flashMessages">
@@ -80,7 +87,7 @@ class FlashMessagesViewHelper extends \TYPO3\CMS\Fluid\Core\ViewHelper\AbstractT
     /**
      * @var string
      */
-    protected $tagName = 'ul';
+    protected $tagName = 'div';
 
     /**
      * Initialize arguments
@@ -114,7 +121,7 @@ class FlashMessagesViewHelper extends \TYPO3\CMS\Fluid\Core\ViewHelper\AbstractT
         }
 
         if ($as === null) {
-            $content = $this->renderAsList($flashMessages);
+            $content = $this->renderDefault($flashMessages);
         } else {
             $content = $this->renderFromTemplate($flashMessages, $as);
         }
@@ -128,19 +135,14 @@ class FlashMessagesViewHelper extends \TYPO3\CMS\Fluid\Core\ViewHelper\AbstractT
      * @param array $flashMessages \TYPO3\CMS\Core\Messaging\FlashMessage[]
      * @return string
      */
-    protected function renderAsList(array $flashMessages)
+    protected function renderDefault(array $flashMessages)
     {
         $flashMessagesClass = $this->hasArgument('class') ? $this->arguments['class'] : 'typo3-messages';
         $tagContent = '';
         $this->tag->addAttribute('class', $flashMessagesClass);
         /** @var $singleFlashMessage \TYPO3\CMS\Core\Messaging\FlashMessage */
         foreach ($flashMessages as $singleFlashMessage) {
-            $severityClass = sprintf('alert %s', $singleFlashMessage->getClass());
-            $messageContent = htmlspecialchars($singleFlashMessage->getMessage());
-            if ($singleFlashMessage->getTitle() !== '') {
-                $messageContent = sprintf('<h4>%s</h4>', htmlspecialchars($singleFlashMessage->getTitle())) . $messageContent;
-            }
-            $tagContent .= sprintf('<li class="%s">%s</li>', htmlspecialchars($severityClass), $messageContent);
+            $tagContent .= $singleFlashMessage->getMessageAsMarkup();
         }
         $this->tag->setContent($tagContent);
         return $this->tag->render();
index d42650b..4da03ca 100644 (file)
@@ -3,26 +3,7 @@
 <f:layout name="Default" />
 
 <f:section name="content">
-       <f:flashMessages as="flashMessages" queueIdentifier="impexp.errors">
-               <f:for each="{flashMessages}" as="flashMessage">
-                       <div class="alert {flashMessage.class}">
-                               <div class="media">
-                                       <div class="media-left">
-                                               <span class="fa-stack fa-lg">
-                                                       <i class="fa fa-circle fa-stack-2x"></i>
-                                                       <i class="fa fa-{flashMessage.iconName} fa-stack-1x"></i>
-                                               </span>
-                                       </div>
-                                       <div class="media-body">
-                                               <f:if condition="{flashMessage.title}">
-                                                       <h4 class="alert-title">{flashMessage.title}</h4>
-                                               </f:if>
-                                               <div class="alert-message">{flashMessage.message}</div>
-                                       </div>
-                               </div>
-                       </div>
-               </f:for>
-       </f:flashMessages>
+       <f:flashMessages queueIdentifier="impexp.errors" />
        <ul class="nav nav-tabs" role="tablist">
                <li role="presentation" class="active">
                        <a class="text-capitalize" href="#import-import" aria-controls="import-import" role="tab" data-toggle="tab">