From 721630f4b29535d8aaf2ac8b97c27e2bb086c9b6 Mon Sep 17 00:00:00 2001
From: Felix Hamann <felix.hamann@campus.lmu.de>
Date: Sat, 20 Apr 2019 22:23:51 +0200
Subject: [PATCH 1/5] add styling for course communication recipient form

---
 .../widgets/communication/recipientAdd.hamlet |  2 +-
 .../communication/recipientEmail.hamlet       |  7 ++--
 .../communication/recipientLayout.hamlet      |  7 ++--
 .../communication/recipientLayout.lucius      | 40 +++++++++++++++++++
 .../communication/recipientName.hamlet        |  4 +-
 5 files changed, 50 insertions(+), 10 deletions(-)
 create mode 100644 templates/widgets/communication/recipientLayout.lucius

diff --git a/templates/widgets/communication/recipientAdd.hamlet b/templates/widgets/communication/recipientAdd.hamlet
index 6f992e6bd..3c99d2964 100644
--- a/templates/widgets/communication/recipientAdd.hamlet
+++ b/templates/widgets/communication/recipientAdd.hamlet
@@ -1,5 +1,5 @@
 $newline never
-<div .category__entry-add>
+<div .recipient-category__option-add>
   #{csrf}
   ^{fvInput addView}
   ^{fvInput submitView}
diff --git a/templates/widgets/communication/recipientEmail.hamlet b/templates/widgets/communication/recipientEmail.hamlet
index ed3964879..5290fcef2 100644
--- a/templates/widgets/communication/recipientEmail.hamlet
+++ b/templates/widgets/communication/recipientEmail.hamlet
@@ -1,7 +1,6 @@
 $newline never
-<div .category__option>
+<div .recipient-category__option>
   #{csrf}
   ^{fvInput tickView}
-  <label for=#{fvId tickView}>
-    <span style="font-family: monospace">
-      #{email}
+  <label .recipient-category__option-label for=#{fvId tickView}>
+    #{email}
diff --git a/templates/widgets/communication/recipientLayout.hamlet b/templates/widgets/communication/recipientLayout.hamlet
index 9f0ff360f..680778623 100644
--- a/templates/widgets/communication/recipientLayout.hamlet
+++ b/templates/widgets/communication/recipientLayout.hamlet
@@ -1,11 +1,12 @@
 $newline never
 $forall category <- activeCategories
-  <div .category>
+  <div .recipient-category>
     <input type=checkbox id=#{checkedIdent category} :elem category checkedCategories:checked>
-    <label for=#{checkedIdent category}>
+    <label .recipient-category__label for=#{checkedIdent category}>
       _{category}
 
-    <fieldset uw-interactive-fieldset data-conditional-input=#{checkedIdent category} .interactive-fieldset--target>
+    $# TODO: only add fieldset if there are options to choose from
+    <fieldset .recipient-category__fieldset uw-interactive-fieldset data-conditional-input=#{checkedIdent category}>
       $forall optIx <- categoryIndices category
         ^{cellWdgts ! optIx}
 
diff --git a/templates/widgets/communication/recipientLayout.lucius b/templates/widgets/communication/recipientLayout.lucius
new file mode 100644
index 000000000..c7636f072
--- /dev/null
+++ b/templates/widgets/communication/recipientLayout.lucius
@@ -0,0 +1,40 @@
+.recipient-category {
+  max-width: 400px;
+  padding: 3px 0;
+
+  &:not(:last-child) {
+    margin-bottom: 7px;
+  }
+
+  &:not(:first-child) {
+    margin-top: 7px;
+  }
+}
+
+.recipient-category__option {
+  display: flex;
+
+  &:not(:last-child) {
+    margin-bottom: 10px;
+  }
+}
+
+.recipient-category__label,
+.recipient-category__option-label {
+  margin-left: 15px;
+  vertical-align: top;
+}
+
+.recipient-category__fieldset {
+  margin: 7px 0 5px 9px;
+  padding: 5px 0 10px;
+  border-left: 1px solid #bcbcbc;
+  padding-left: 16px;
+  max-height: 200px;
+  overflow-y: auto;
+}
+
+.recipient-category__option-add {
+  display: flex;
+  align-items: center;
+}
diff --git a/templates/widgets/communication/recipientName.hamlet b/templates/widgets/communication/recipientName.hamlet
index 1da8a7fe3..96b650b0d 100644
--- a/templates/widgets/communication/recipientName.hamlet
+++ b/templates/widgets/communication/recipientName.hamlet
@@ -1,6 +1,6 @@
 $newline never
-<div .category__option>
+<div .recipient-category__option>
   #{csrf}
   ^{fvInput tickView}
-  <label for=#{fvId tickView}>
+  <label .recipient-category__option-label for=#{fvId tickView}>
     #{nameHtml userDisplayName userSurname}

From b35dc87db6d05e514ac787c19dda1e279e741b12 Mon Sep 17 00:00:00 2001
From: Felix Hamann <felix.hamann@campus.lmu.de>
Date: Sat, 20 Apr 2019 22:24:50 +0200
Subject: [PATCH 2/5] make target for interactive-fieldset optional

---
 static/js/utils/form.js              | 13 +++----------
 templates/widgets/aform/aform.hamlet |  2 +-
 2 files changed, 4 insertions(+), 11 deletions(-)

diff --git a/static/js/utils/form.js b/static/js/utils/form.js
index b2a449e2a..ad87b3501 100644
--- a/static/js/utils/form.js
+++ b/static/js/utils/form.js
@@ -182,15 +182,8 @@
       }
       conditionalValue = element.dataset.conditionalValue;
 
-      if (element.matches(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR)) {
-        target = element;
-      }
-      if (!target) {
-        target = element.closest(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR);
-      }
-      if (!target) {
-        throw new Error('Interactive Fieldset needs to be a target or have a target-ancestor!');
-      }
+      // check if there is a target ancestor, otherwise target the fieldset itself
+      target = element.closest(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR) || element;
 
       childInputs = Array.from(element.querySelectorAll(INTERACTIVE_FIELDSET_CHILD_SELECTOR));
 
@@ -212,7 +205,7 @@
 
     function updateVisibility() {
       var active = matchesConditionalValue();
-      
+
       target.classList.toggle('hidden', !active);
 
       childInputs.forEach(function(el) {
diff --git a/templates/widgets/aform/aform.hamlet b/templates/widgets/aform/aform.hamlet
index 00f8755f2..91ebc2b93 100644
--- a/templates/widgets/aform/aform.hamlet
+++ b/templates/widgets/aform/aform.hamlet
@@ -11,7 +11,7 @@ $case formLayout
         <h3 .form-section-title>
           ^{fvLabel view}
       $else
-        <div .form-group .interactive-fieldset--target :fvRequired view:.form-group--required :not $ fvRequired view:.form-group--optional :isJust $ fvErrors view:.form-group--has-error>
+        <div .form-group :fvRequired view:.form-group--required :not $ fvRequired view:.form-group--optional :isJust $ fvErrors view:.form-group--has-error>
           $if not (Blaze.null $ fvLabel view)
             <label .form-group-label for=#{fvId view}>
               <span .form-group-label__caption>

From b74de0f88df511b8d3c47890829d7a06ced8766e Mon Sep 17 00:00:00 2001
From: Felix Hamann <felix.hamann@campus.lmu.de>
Date: Sun, 21 Apr 2019 17:22:22 +0200
Subject: [PATCH 3/5] reintroduce explicit interactive fieldset target but keep
 it optional

---
 static/js/utils/form.js                                | 8 +++++---
 templates/widgets/aform/aform.hamlet                   | 2 +-
 templates/widgets/communication/recipientLayout.hamlet | 2 +-
 3 files changed, 7 insertions(+), 5 deletions(-)

diff --git a/static/js/utils/form.js b/static/js/utils/form.js
index 7a949c539..a68f56b9f 100644
--- a/static/js/utils/form.js
+++ b/static/js/utils/form.js
@@ -146,7 +146,7 @@
 
   var INTERACTIVE_FIELDSET_UTIL_NAME = 'interactiveFieldset';
   var INTERACTIVE_FIELDSET_UTIL_SELECTOR = '[uw-interactive-fieldset]';
-  var INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR = '.interactive-fieldset--target';
+  var INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR = '.interactive-fieldset__target';
 
   var INTERACTIVE_FIELDSET_INITIALIZED_CLASS = 'interactive-fieldset--initialized';
   var INTERACTIVE_FIELDSET_CHILD_SELECTOR = 'input:not([disabled]), select:not([disabled]), textarea:not([disabled]), button:not([disabled])';
@@ -182,8 +182,10 @@
       }
       conditionalValue = element.dataset.conditionalValue;
 
-      // check if there is a target ancestor, otherwise target the fieldset itself
-      target = element.closest(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR) || element;
+      target = element.closest(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR);
+      if (!target || element.matches(INTERACTIVE_FIELDSET_UTIL_TARGET_SELECTOR)) {
+        target = element;
+      }
 
       childInputs = Array.from(element.querySelectorAll(INTERACTIVE_FIELDSET_CHILD_SELECTOR));
 
diff --git a/templates/widgets/aform/aform.hamlet b/templates/widgets/aform/aform.hamlet
index 91ebc2b93..460ca1ba7 100644
--- a/templates/widgets/aform/aform.hamlet
+++ b/templates/widgets/aform/aform.hamlet
@@ -11,7 +11,7 @@ $case formLayout
         <h3 .form-section-title>
           ^{fvLabel view}
       $else
-        <div .form-group :fvRequired view:.form-group--required :not $ fvRequired view:.form-group--optional :isJust $ fvErrors view:.form-group--has-error>
+        <div .form-group .interactive-fieldset__target :fvRequired view:.form-group--required :not $ fvRequired view:.form-group--optional :isJust $ fvErrors view:.form-group--has-error>
           $if not (Blaze.null $ fvLabel view)
             <label .form-group-label for=#{fvId view}>
               <span .form-group-label__caption>
diff --git a/templates/widgets/communication/recipientLayout.hamlet b/templates/widgets/communication/recipientLayout.hamlet
index 680778623..3fbdddad4 100644
--- a/templates/widgets/communication/recipientLayout.hamlet
+++ b/templates/widgets/communication/recipientLayout.hamlet
@@ -6,7 +6,7 @@ $forall category <- activeCategories
       _{category}
 
     $# TODO: only add fieldset if there are options to choose from
-    <fieldset .recipient-category__fieldset uw-interactive-fieldset data-conditional-input=#{checkedIdent category}>
+    <fieldset .recipient-category__fieldset uw-interactive-fieldset .interactive-fieldset__target data-conditional-input=#{checkedIdent category}>
       $forall optIx <- categoryIndices category
         ^{cellWdgts ! optIx}
 

From 07e8b5e203dcd6c05cbfe3a2da0176ff07a571ba Mon Sep 17 00:00:00 2001
From: Gregor Kleen <kleen@cip.ifi.lmu.de>
Date: Sun, 21 Apr 2019 17:36:52 +0200
Subject: [PATCH 4/5] Only include <fieldset> if recipient category has content

---
 src/Handler/Utils/Communication.hs                   |  1 +
 .../widgets/communication/recipientLayout.hamlet     | 12 ++++++------
 2 files changed, 7 insertions(+), 6 deletions(-)

diff --git a/src/Handler/Utils/Communication.hs b/src/Handler/Utils/Communication.hs
index 1ccf45abc..d0ebfb186 100644
--- a/src/Handler/Utils/Communication.hs
+++ b/src/Handler/Utils/Communication.hs
@@ -158,6 +158,7 @@ commR CommunicationRoute{..} =  do
             checkedIdentBase <- newIdent
             let checkedCategories = Set.mapMonotonic (unEnumPosition . fst) . Set.filter (\k' -> Map.foldrWithKey (\k (_, checkState) -> (||) $ k == k' && checkState /= FormSuccess False && (checkState /= FormMissing || maybe True snd (chosenRecipients' !? k))) False state) $ Map.keysSet state
                 checkedIdent c = checkedIdentBase <> "-" <> toPathPiece c
+                hasContent c = not (null $ categoryIndices c) || Map.member (1, (EnumPosition c, 0)) addWdgts
                 categoryIndices c = Set.filter ((== c) . unEnumPosition . fst) $ review liveCoords liveliness
             $(widgetFile "widgets/communication/recipientLayout")
           miDelete :: MapLiveliness (EnumLiveliness RecipientCategory) ListLength -> (EnumPosition RecipientCategory, ListPosition) -> MaybeT (MForm Handler) (Map (EnumPosition RecipientCategory, ListPosition) (EnumPosition RecipientCategory, ListPosition))
diff --git a/templates/widgets/communication/recipientLayout.hamlet b/templates/widgets/communication/recipientLayout.hamlet
index 3fbdddad4..c52ff534d 100644
--- a/templates/widgets/communication/recipientLayout.hamlet
+++ b/templates/widgets/communication/recipientLayout.hamlet
@@ -5,10 +5,10 @@ $forall category <- activeCategories
     <label .recipient-category__label for=#{checkedIdent category}>
       _{category}
 
-    $# TODO: only add fieldset if there are options to choose from
-    <fieldset .recipient-category__fieldset uw-interactive-fieldset .interactive-fieldset__target data-conditional-input=#{checkedIdent category}>
-      $forall optIx <- categoryIndices category
-        ^{cellWdgts ! optIx}
+    $if hasContent category
+      <fieldset .recipient-category__fieldset uw-interactive-fieldset .interactive-fieldset__target data-conditional-input=#{checkedIdent category}>
+        $forall optIx <- categoryIndices category
+          ^{cellWdgts ! optIx}
 
-      $maybe addWdgt <- addWdgts !? (1, (EnumPosition category, 0))
-        ^{addWdgt}
+        $maybe addWdgt <- addWdgts !? (1, (EnumPosition category, 0))
+          ^{addWdgt}

From 5dda5aa1bab0278e3860fd281d730d8b94eb81f9 Mon Sep 17 00:00:00 2001
From: Felix Hamann <felix.hamann@campus.lmu.de>
Date: Sun, 21 Apr 2019 18:14:01 +0200
Subject: [PATCH 5/5] adjust spacing for mass-input-add button in course
 communication

---
 templates/widgets/communication/recipientLayout.lucius | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/templates/widgets/communication/recipientLayout.lucius b/templates/widgets/communication/recipientLayout.lucius
index c7636f072..df68114dd 100644
--- a/templates/widgets/communication/recipientLayout.lucius
+++ b/templates/widgets/communication/recipientLayout.lucius
@@ -36,5 +36,9 @@
 
 .recipient-category__option-add {
   display: flex;
-  align-items: center;
+
+  .btn-mass-input-add {
+    margin-left: 10px;
+    padding: 10px 0;
+  }
 }