From 66905513825c23b29807ebcf0bdee406e107dd18 Mon Sep 17 00:00:00 2001
From: Colleen Palmer <colleen@instructure.com>
Date: Wed, 4 May 2016 17:29:54 -0600
Subject: [PATCH] User edit icon not clickable in small windows

closes: CNVS-26991

Fix was to consolidate buttons into dropdown vs right sidebar

Test plan:
- As an admin/teacher go to People in a course, there should be a cog
dropdown with what used to be the right side buttons
- As a student go to a course and view people, you should see the links
associated with that role in the dropdown

Change-Id: Ie3661468165be9ef71c9533d73a7c99ae9bf7cd1
Reviewed-on: https://gerrit.instructure.com/78814
Reviewed-by: Clay Diffrient <cdiffrient@instructure.com>
Tested-by: Jenkins
QA-Review: Nathan Rogowski <nathan@instructure.com>
Product-Review: Colleen Palmer <colleen@instructure.com>
---
 app/helpers/application_helper.rb             |  2 +-
 app/views/context/_roster_right_side.html.erb | 78 +++++++++++--------
 app/views/context/roster.html.erb             | 11 ++-
 .../helpers/groups_shared_examples.rb         |  6 +-
 spec/selenium/people_spec.rb                  | 33 ++++++--
 5 files changed, 85 insertions(+), 45 deletions(-)

diff --git a/app/helpers/application_helper.rb b/app/helpers/application_helper.rb
index de5a167a3c9..32bc151f9dc 100644
--- a/app/helpers/application_helper.rb
+++ b/app/helpers/application_helper.rb
@@ -414,7 +414,7 @@ module ApplicationHelper
   #
   # Returns an HTML string.
   def sidebar_button(url, label, img = nil)
-    link_to(url, :class => 'btn button-sidebar-wide') do
+    link_to(url) do
       img ? ("<i class='icon-" + img + "'></i> ").html_safe + label : label
     end
   end
diff --git a/app/views/context/_roster_right_side.html.erb b/app/views/context/_roster_right_side.html.erb
index ea101a0b9c7..67e49093da0 100644
--- a/app/views/context/_roster_right_side.html.erb
+++ b/app/views/context/_roster_right_side.html.erb
@@ -1,36 +1,48 @@
-<div class="rs-margin-lr <% unless use_new_styles? %>rs-margin-top<% end %>">
-  <% if @context.is_a?(Course) %>
+<div id="people-options" class="al-dropdown__container" style="position: relative;">
+  <button type="button" class="al-trigger Button">
+    <i class="icon-settings"></i>
+    <i class="icon-mini-arrow-down"></i>
+    <span class="screenreader-only">More Options</span>
+  </button>
+  <ul class="al-options" role="menu" tabindex="0" aria-hidden="true">
+    <% if @context.is_a?(Course) %>
+      <% if can_do(@context, @current_user, :read_roster) %>
+        <li role="presentation">
+        <%= sidebar_button context_url(@context, :context_groups_url),
+                           t('View User Groups'),
+                           'group' %>
+        </li>
+      <% end %>
+
+      <% if can_do(@context, @current_user, :manage_students && @context.enable_user_notes) %>
+        <li role="presentation">
+        <%= sidebar_button course_user_notes_path(@context),
+                           t('View Faculty Journals') %>
+        </li>
+      <% end %>
+
+      <% if can_do(@context, @current_user, :manage_admin_users, :manage_students, :read_prior_roster) %>
+        <li role="presentation">
+        <%= sidebar_button course_prior_users_path(@context),
+                           t('View Prior Enrollments'),
+                           'clock' %>
+        </li>
+      <% end %>
+      <% if @context.user_is_instructor?(@current_user) && can_do(@context, @current_user, :read_reports) %>
+        <li role="presentation">
+        <%= sidebar_button user_course_teacher_activity_url(@current_user, @context),
+                           t('Student Interactions Report'),
+                           'user' %>
+        </li>
+      <% end %>
+    <% end %>
+
     <% if can_do(@context, @current_user, :read_roster) %>
-      <%= sidebar_button context_url(@context, :context_groups_url),
-                         t('links.view_user_groups', 'View User Groups'),
-                         'group' %>
+      <li role="presentation">
+      <%= sidebar_button context_url(@context, :context_user_services_url),
+                         t('links.view_services', 'View Registered Services'),
+                         'link' %>
+      </li>
     <% end %>
-
-    <% if can_do(@context, @current_user, :manage_students && @context.enable_user_notes) %>
-      <%= sidebar_button course_user_notes_path(@context),
-                         t('links.view_faculty_journals', 'View Faculty Journals') %>
-    <% end %>
-
-    <% if can_do(@context, @current_user, :manage_admin_users, :manage_students, :read_prior_roster) %>
-      <%= sidebar_button course_prior_users_path(@context),
-                         t('links.view_prior_enrollments', 'View Prior Enrollments'),
-                         'clock' %>
-    <% end %>
-  <% end %>
-
-  <% if can_do(@context, @current_user, :read_roster) %>
-    <%= sidebar_button context_url(@context, :context_user_services_url),
-                       t('links.view_services', 'View Registered Services'),
-                       'link' %>
-  <% end %>
-
-  <% if @context.is_a?(Course) %>
-    <% if @context.user_is_instructor?(@current_user) && can_do(@context, @current_user, :read_reports) %>
-      <%= sidebar_button user_course_teacher_activity_url(@current_user, @context),
-                         t('Student Interactions Report'),
-                         'user' %>
-    <% end %>
-  <% end %>
-
+  </ul>
 </div>
-
diff --git a/app/views/context/roster.html.erb b/app/views/context/roster.html.erb
index 70c2d2e8f44..990b6262196 100644
--- a/app/views/context/roster.html.erb
+++ b/app/views/context/roster.html.erb
@@ -6,12 +6,19 @@
   content_for :page_title, join_title(translated_title, @context.name)
 %>
 
-<h1 class='screenreader-only'><%= t('#titles.people', 'People') %></h1>
 
-<% content_for :right_side, render(:partial => 'context/roster_right_side') %>
 
 <% css_bundle :roster %>
 
+
+<div class="ic-Action-header">
+  <div class="ic-Action-header__Primary">
+    <h1 class="screenreader-only"><%= t('#titles.people', 'People') %></h1>
+  </div>
+  <div class="ic-Action-header__Secondary">
+    <%= render :partial => 'context/roster_right_side' %>
+  </div>
+</div>
 <% if @context.is_a?(Course) %>
   <% js_bundle :roster %>
   <% js_env :canManageCourse => can_do(@context, @current_user, :manage) %>
diff --git a/spec/selenium/helpers/groups_shared_examples.rb b/spec/selenium/helpers/groups_shared_examples.rb
index 1d742942991..b5b74321332 100644
--- a/spec/selenium/helpers/groups_shared_examples.rb
+++ b/spec/selenium/helpers/groups_shared_examples.rb
@@ -141,7 +141,10 @@ end
 shared_examples 'people_page' do |context|
   it "should allow group users to see group registered services page", priority: pick_priority(context, student: "1", teacher: "2"),test_id: pick_test_id(context, student: 323329, teacher: 324926) do
     get people_page
-    expect_new_page_load { fln('View Registered Services').click }
+    expect_new_page_load do
+      f("#people-options .Button").click
+      fln('View Registered Services').click
+    end
     # Checks that we are on the Registered Services page
     expect(f('.btn.button-sidebar-wide')).to be_displayed
   end
@@ -235,4 +238,3 @@ shared_examples 'conferences_page' do |context|
     expect(f('#concluded-conference-list')).to include_text('There are no concluded conferences')
   end
 end
-
diff --git a/spec/selenium/people_spec.rb b/spec/selenium/people_spec.rb
index 62d41c61071..4b555bdcde0 100755
--- a/spec/selenium/people_spec.rb
+++ b/spec/selenium/people_spec.rb
@@ -22,7 +22,10 @@ describe "people" do
   end
 
   def create_student_group(group_text = "new student group")
-    expect_new_page_load { fln('View User Groups').click }
+    expect_new_page_load do
+      f("#people-options .Button").click
+      fln('View User Groups').click
+    end
     open_student_group_dialog
     inputs = ffj('input:visible')
     replace_content(inputs[0], group_text)
@@ -167,6 +170,7 @@ describe "people" do
     end
 
     it "should display activity report on clicking Student Interaction button", priority: "1", test_id: 244446 do
+      f("#people-options .Button").click
       fln("Student Interactions Report").click
       wait_for_ajaximations
       user_name = f(".user_name").text
@@ -204,6 +208,7 @@ describe "people" do
     end
 
     it "should navigate to registered services on profile page" do
+      f("#people-options .Button").click
       fln('View Registered Services').click
       fln('Link web services to my account').click
       expect(f('#unregistered_services')).to be_displayed
@@ -214,6 +219,7 @@ describe "people" do
     end
 
     it "should test self sign up help functionality" do
+      f("#people-options .Button").click
       expect_new_page_load { fln('View User Groups').click }
       open_student_group_dialog
       fj('.self_signup_help_link:visible').click
@@ -222,6 +228,7 @@ describe "people" do
     end
 
     it "should test self sign up functionality" do
+      f("#people-options .Button").click
       expect_new_page_load { fln('View User Groups').click }
       dialog = open_student_group_dialog
       dialog.find_element(:css, '#category_enable_self_signup').click
@@ -231,7 +238,10 @@ describe "people" do
 
     it "should test self sign up / group structure functionality" do
       group_count = "4"
-      expect_new_page_load { fln('View User Groups').click }
+      expect_new_page_load do
+        f("#people-options .Button").click
+        fln('View User Groups').click
+      end
       dialog = open_student_group_dialog
       dialog.find_element(:css, '#category_enable_self_signup').click
       dialog.find_element(:css, '#category_create_group_count').send_keys(group_count)
@@ -245,7 +255,10 @@ describe "people" do
       enroll_more_students
 
       group_count = "4"
-      expect_new_page_load { fln('View User Groups').click }
+      expect_new_page_load do
+        f("#people-options .Button").click
+        fln('View User Groups').click
+      end
       dialog = open_student_group_dialog
       dialog.find_element(:css, '#category_split_groups').click
       replace_content(f('#category_split_group_count'), group_count)
@@ -286,7 +299,10 @@ describe "people" do
       enroll_more_students
 
       group_count = 4
-      expect_new_page_load { fln('View User Groups').click }
+      expect_new_page_load do
+        f("#people-options .Button").click
+        fln('View User Groups').click
+      end
       open_student_group_dialog
       submit_form('#add_category_form')
       wait_for_ajaximations
@@ -309,7 +325,10 @@ describe "people" do
     it "should test prior enrollment functionality" do
       @course.complete
       get "/courses/#{@course.id}/users"
-      expect_new_page_load { fln('View Prior Enrollments').click }
+      expect_new_page_load do
+        f("#people-options .Button").click
+        fln('View Prior Enrollments').click
+      end
       expect(f('#users')).to include_text(@student_1.name)
     end
 
@@ -399,7 +418,7 @@ describe "people" do
       student = create_user("student@example.com")
       enroll_student(student)
       get "/courses/#{@course.id}/users"
-      ff(".icon-settings")[1].click
+      f(".StudentEnrollment .icon-settings").click
       fln("Edit Sections").click
       f(".token_input.browsable").click
       section_input_element = driver.find_element(:name, "token_capture")
@@ -416,7 +435,7 @@ describe "people" do
      @course.enroll_student(@student, allow_multiple_enrollments: true)
      @course.enroll_student(@student, section: @section2, allow_multiple_enrollments: true)
      get "/courses/#{@course.id}/users"
-     ff(".icon-settings")[1].click
+     f(".StudentEnrollment .icon-settings").click
      fln("Edit Sections").click
      fln("Remove user from section2").click
      ff('.ui-button-text')[1].click