305 - UI fixes to add author partial (#385)

* Fix unnecessary 'required' marks, 2-col format

* Make author form two-column format

* rebase

* Text horizontal alignment of items

* Move public function out of private section

* re-do what was accidentally undone with rebase

app/assets/javascripts/add_remove_authors.js

@@ -55,7 +55,7 @@ function addAuthor(type) {
 
 function createAuthorElement(type) {
     const newDataIndex = document.querySelectorAll(`[data-type='${type}']`).length;
-    const newAuthor = createElementWithAttributes("div", { class: "form-row", 'data-type': type, 'data-index': newDataIndex });
+    const newAuthor = createElementWithAttributes("div", { class: "form-row form-container author-form-container", 'data-type': type, 'data-index': newDataIndex, 'id': 'author_' + newDataIndex });
     newAuthor.appendChild(createInput(type, "author_first_name"));
     newAuthor.appendChild(createInput(type, "author_last_name"));
     newAuthor.appendChild(createDeleteButton());
@@ -69,18 +69,15 @@ function createElementWithAttributes(tag, attributes) {
 }
 
 function createInput(type, name) {
-    // The "for" attribute of the label will be updated with the updateAuthorIds function.
-
     const role = getRoleFromButton();
     if (!role) return;
 
     const wrapper = createElementWithAttributes("div", {
-        class: "col-md-5",
+        class: "form-item",
         "data-type": type
     });
 
     const label = createElementWithAttributes("label", {
-        class: "required",
         "data-type": type
     });
 
@@ -105,7 +102,7 @@ function createDeleteButton() {
     const role = getRoleFromButton();
     if (!role) return;
 
-    const wrapper = createElementWithAttributes("div", { class: "col-md-2" });
+    const wrapper = createElementWithAttributes("div", { class: "form-delete-button col-3" });
     const button = createElementWithAttributes("button", {
         type: "button",
         class: "form-control form-group bg-danger text-white"

app/assets/stylesheets/forms.scss

@@ -1,37 +1,63 @@
-.two-column-form {
+.form-container {
   display: flex;
-  flex-direction: column;
+  flex-wrap: wrap;
+  align-items: flex-start;
 
-  input, select {
-    border: 1px solid #adb5bd; /* Default state */
+  .form-item {
+    flex: 1 1 100%; // Full width on mobile
+    justify-content: space-between;
+    margin-right: 10px;
+    margin-top: 10px;
+    @media (min-width: 768px) { // Tablet and up
+      flex: 1 1 calc(50% - 20px); // Two column layout
+    }
   }
+}
 
-  input:focus, select:focus {
-    border-color: #6c757d; /* Darker border on focus */
-    outline: none; /* Removes the default focus outline to rely on the border change */
+.author-form-container {
+  .form-item {
+    @media (min-width: 992px) { // Desktop and up
+      flex: 1 1 calc((100% - 30px) / 3); // Three columns, assuming a 10px margin
+    }
   }
 
-
-  // Tablets and above need two columns, mobile needs one
-  @media (min-width: 768px) {
-    flex-direction: row;
-    flex-wrap: wrap;
-    justify-content: space-between;
+  // An invisible delete button is used for aligning items in the author form
+  .invisible {
+    display: none;
+    @media (min-width: 992px) {
+      display: block;
+    }
   }
 
-  .two-column-item {
-    flex: 1 1 auto;
-    margin: 10px;
-    @media (min-width: 768px) {
-      flex: 0 1 45%;
+  .form-delete-button {
+    flex: 0 0 auto;
+    margin-top: 10px;
+    @media (min-width: 992px) {
+      align-self: center;
+      margin-left: auto;
+      margin-top: auto;
+      width: auto;
     }
   }
+}
 
-  .add-padding-bottom {
-    padding-bottom: 1.5rem;
+// Adds a ghost/filler item at the end of the .form-container to preserve the layout
+.form-container::after {
+  content: '';
+  flex: 1 1 100%;
+  @media (min-width: 768px) {
+      flex: 1 1 calc(50% - 10px);
   }
 }
 
-.two-column-left-button {
-  margin-left: 10px;
-}
+
+
+.add-padding-bottom {
+  padding-bottom: 1.5rem;
+}
+
+/* Draw a line between each author inside author-group */
+.author_group > :not(:last-child) {
+  border-bottom: 1px solid #ccc;
+  padding-bottom: 10px;
+}

app/views/layouts/application.html.erb

@@ -1,5 +1,5 @@
 <!DOCTYPE html>
-<html>
+<html lang="en">
   <head>
     <title>Artists, Authors, Editors & Composers | UC Libraries</title>
     <%= csrf_meta_tags %>

app/views/partials/_author.html.erb

@@ -1,27 +1,25 @@
 
-<div id="author_group">
+<div id="author_group" class="author_group">
   <% if publication.author_first_name.empty? && publication.author_last_name.empty? %>
     <% publication.author_first_name << '' %>
     <% publication.author_last_name << '' %>
   <% end %>
 
   <% publication.author_first_name.zip(publication.author_last_name).each_with_index do |(first_name, last_name), index| %>
-    <div class="form-row" data-type="<%= name %>" data-index=<%= index %> id="author_<%= index %>">
-      <div class="col-md-5">
+    <div class="form-row form-container author-form-container" data-type="<%= name %>" data-index=<%= index %> id="author_<%= index %>">
+      <div class="col-md-5 form-item">
         <%= label_tag("author_first_name_#{index}", "#{author_or_artist_label} First Name", class: "required") %>
         <%= text_field_tag "#{name}[author_first_name][]", first_name, "data-publication-type": name, id: "author_first_name_#{index}", autocomplete: 'off', required: true, class: "form-control form-group" %>
       </div>
-      <div class="col-md-5">
+      <div class="col-md-5 form-item">
         <%= label_tag "author_last_name_#{index}", "#{author_or_artist_label} Last Name", class: "required" %>
         <%= text_field_tag "#{name}[author_last_name][]", last_name, "data-publication-type": name, id: "author_last_name_#{index}", autocomplete: 'off', required: true, class: "form-control form-group" %>
       </div>
-      <% if index > 0 %>
-        <div class="col-md-2">
-          <%= button_tag "Remove " + author_or_artist_label, type: "button", class: "form-control form-group bg-danger text-white" %>
-        </div>
-      <% end %>
+      <div class="<%= index > 0 ? 'form-delete-button' : 'form-delete-button col-2 invisible'%>">
+        <%= button_tag "Remove " + author_or_artist_label, type: "button", class: "form-control form-group bg-danger text-white" %>
+      </div>
     </div>
   <% end %>
 </div>
 
-<%= button_tag "Add " + author_or_artist_label, type: "button", id: "add_author_btn", onclick: "addAuthor('#{name}')", class: "btn btn-primary" %>
+<%= button_tag "Add " + author_or_artist_label, type: "button", id: "add_author_btn", onclick: "addAuthor('#{name}')", class: "btn btn-primary mt-3 col-3" %>

app/views/submitters/_form.html.erb

@@ -9,45 +9,45 @@
     </div>
   <% end %>
 
-  <div class="form-row two-column-form">
-    <div class="form-group two-column-item add-padding-bottom">
+  <div class="form-row form-container">
+    <div class="form-group form-item add-padding-bottom">
       <%= form.label :first_name, 'First Name', class: "required" %>
       <%= form.text_field :first_name, required: true, class: "form-control" %>
     </div>
 
-    <div class="form-group two-column-item add-padding-bottom">
+    <div class="form-group form-item add-padding-bottom">
       <%= form.label :last_name, 'Last Name', class: "required" %>
       <%= form.text_field :last_name, required: true, class: "form-control" %>
     </div>
 
-    <div class="form-group two-column-item">
+    <div class="form-group form-item">
       <%= form.label(:college, "UC College") %>
       <%= select_tag "submitter[college]", options_from_collection_for_select(College.all, :id, :name, @submitter.college), prompt: "Please select a college", class: "form-control" %>
       <small class="form-text text-muted">If "Other", enter your college and department in the UC Department field</small>
     </div>
 
-    <div class="form-group two-column-item add-padding-bottom">
+    <div class="form-group form-item">
       <%= form.label(:department, "UC Department or Division") %>
       <%= form.text_field :department, class: "form-control" %>
     </div>
 
-    <div class="form-group two-column-item add-padding-bottom">
+    <div class="form-group form-item">
       <%= form.label(:mailing_address, "Mail Location (Postal address if off campus)", class: "required") %>
       <%= form.text_field :mailing_address, required: true, class: "form-control" %>
     </div>
 
-    <div class="form-group two-column-item">
+    <div class="form-group form-item">
       <%= form.label :phone_number, 'Phone Number', class: "required" %>
       <%= form.text_field :phone_number, required: true, class: "form-control" %>
       <small class="form-text text-muted">Must be in the form ###-###-####</small>
     </div>
 
-    <div class="form-group two-column-item add-padding-bottom">
+    <div class="form-group form-item">
       <%= form.label(:email_address, "Email Address", class: "required") %>
       <%= form.text_field :email_address, required: true, class: "form-control" %>
     </div>
   </div>
 
-  <%= form.submit "Next", class: "btn btn-primary two-column-left-button" %>
+  <%= form.submit "Next", class: "btn btn-primary mt-4 col-2" %>
 
 <% end %>

spec/features/layouts/form_layout_spec.rb

@@ -0,0 +1,149 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe 'Form Layout Test', type: :feature, js: true do
+  let(:submitter) { FactoryBot.build(:submitter) }
+
+  context 'while entering in information as a new submitter' do
+    it 'adjusts layout from one to two columns based on screen width' do
+      visit root_path
+      mobile_has_one_column
+      tablet_has_two_columns
+      desktop_has_two_columns
+    end
+  end
+
+  context 'while adding a new author' do
+    it 'adjusts layout from one to two columns based on screen width' do
+      create_submitter(submitter)
+      visit new_book_path
+      click_button('Add Author')
+      click_button('Add Author')
+      mobile_has_one_column
+      tablet_has_two_columns
+      desktop_has_three_columns
+    end
+  end
+
+  def container_width
+    page.evaluate_script("document.querySelector('.form-container').offsetWidth").to_i
+  end
+
+  def delete_button_width
+    page.evaluate_script("document.querySelector('.form-delete-button') ? document.querySelector('.form-delete-button').offsetWidth : 0").to_i
+  end
+
+  def mobile_has_one_column
+    # Mobile view: Expect items to take full container width, indicating a one-column layout
+    resize_window_to_mobile
+
+    verify_one_column_widths
+    verify_one_column_heights
+  end
+
+  def tablet_has_two_columns
+    # Tablet view: Expect items to take about half the container width, indicating a two-column layout
+    # The delete button on author form-items will be on a separate line so not counted in the width.
+    resize_window_to_tablet
+
+    verify_two_column_item_widths
+    verify_two_column_item_heights
+  end
+
+  def desktop_has_two_columns
+    # Desktop view: Expect items to take half container width, indicating a two-column layout
+    # There is no delete button, so it is not counted in the width.
+    resize_window_to_desktop
+
+    verify_two_column_item_widths
+    verify_two_column_item_heights
+  end
+
+  def desktop_has_three_columns
+    # Desktop view: Each form-item should take up half of the
+    # container width, minus the width of the delete button
+    resize_window_to_desktop
+    click_button('Add Author')
+
+    verify_three_column_item_widths
+    verify_three_column_item_heights
+  end
+
+  def verify_one_column_widths
+    expected_item_width = container_width - 10 # Subtract padding
+
+    all('.form-item').each do |item|
+      item_width = page.evaluate_script('arguments[0].offsetWidth', item.native).to_i
+      expect(item_width).to be_within(5).of(expected_item_width)
+    end
+  end
+
+  def verify_one_column_heights
+    heights = retrieve_item_offsets
+
+    expect(heights[0]).not_to eq(heights[1])
+  end
+
+  def verify_two_column_item_widths
+    expected_item_width = calculate_expected_item_width(intake_field_count: 2, gap_count: 1)
+
+    all('.form-item').each do |item|
+      item_width = page.evaluate_script('arguments[0].offsetWidth', item.native).to_i
+
+      expect(item_width).to be_within(5).of(expected_item_width)
+    end
+  end
+
+  def verify_two_column_item_heights
+    heights = retrieve_item_offsets
+    expect(heights[0]).to eq(heights[1])
+    expect(heights[1]).not_to eq(heights[2])
+  end
+
+  def verify_three_column_item_widths
+    expected_item_width = calculate_expected_item_width(intake_field_count: 2, gap_count: 2)
+
+    all('.form-item').each do |item|
+      item_width = page.evaluate_script('arguments[0].offsetWidth', item.native).to_i
+
+      expect(item_width).to be_within(5).of(expected_item_width)
+    end
+  end
+
+  def verify_three_column_item_heights
+    heights = retrieve_item_offsets
+
+    expect(heights[0..2].uniq.size).to eq(1)
+    expect(heights[2]).not_to eq(heights[3])
+  end
+
+  # Returns an array of the heights (y-values) of the bottoms of each of the children
+  # for the form-container.
+  # This is used to determine if the form-items are on the same line or not.
+  # I am using the bottom because the heights of the items are not consistent
+  # and they are aligned by their bottoms.
+  def retrieve_item_offsets
+    script = "  Array.from(document.querySelector('.form-container').children).map(function(child) {return child.offsetTop + child.clientHeight; });"
+    page.evaluate_script(script)
+  end
+
+  def calculate_expected_item_width(intake_field_count:, gap_count: 0, space_between_items: 10)
+    total_gap_width = space_between_items * gap_count
+    available_width_for_items = container_width - total_gap_width - delete_button_width
+
+    available_width_for_items / intake_field_count
+  end
+
+  def resize_window_to_mobile
+    page.driver.browser.manage.window.resize_to(360, 640) # Mobile dimensions
+  end
+
+  def resize_window_to_tablet
+    page.driver.browser.manage.window.resize_to(768, 1024) # Tablet dimensions
+  end
+
+  def resize_window_to_desktop
+    page.driver.browser.manage.window.resize_to(1024, 768) # Desktop dimensions
+  end
+end