rubric criterion ui-changes

R12: put "+" after edit links for a11y.
R13: show delete for outcomes after
     rubric update.
R14: put "+" after description / pts
     for a11y.
R15: text for screenreader for "+"
R16: switch to aria label rather than
     screenreader span, due to safari
     issue.  Fixed existing issue that
     incorrectly allowed user to delete
     last cell after update rubric.
R17: bug fixed in R16 was also
     impacting the first cell after
     rubric update.  Fixed that too.

Fixes:  PFS-8517

Test Plan:

Verify that rubric is functionality is intact and
that ui adheres to prototype in all supported
browsers.

Explicitly -
1) Adding a new rating is accomplished by clicking
   "plus" buttons between ratings, not by hovering
   over cell boundary.
2) edit/delete buttons are constant while in edit
   mode, no hover logic needed.
3) delete column has been removed, and delete icon
   is now in criterion description.  Verify that
   learning outcome criterions function right in
   this regard (they should have delete option,
   but not edit).
4) minor css stuff like dashed border.

Change-Id: Ie8abd4b45f9f2e660f8ce18104d6ec006d2a969f
Reviewed-on: https://gerrit.instructure.com/124636
Reviewed-by: Chris Hart <chart@instructure.com>
QA-Review: Aiona Hernandez <ahernandez@instructure.com>
Tested-by: Jenkins
QA-Review: Andrew Porter <hporter-c@instructure.com>
Product-Review: Sidharth Oberoi <soberoi@instructure.com>
This commit is contained in:
Mark Valentine 2017-08-31 10:21:36 -06:00
parent c1df2f0d10
commit 2fb5e49ff0
5 changed files with 99 additions and 116 deletions

View File

@ -38,31 +38,54 @@
.rubric_container {
margin-top: 20px;
tr.criterion .links {
position: absolute;
bottom: 0;
right: 0;
display: none;
float: right;
text-align: right;
}
&.editing {
tr.criterion .links {
display: inline-block;
i {
visibility: hidden;
}
}
td.rating:hover, td.criterion_description:hover, .links a:focus {
i {
visibility: visible;
}
tr.criterion .description {
clear: both;
}
label[for=rubric-title] {
font-weight: bold;
}
tr.criterion .container .add_rating_link {
position: absolute;
float: right;
top: 50%;
right: -.5%;
transform: translateY(-50%);
line-height: 0;
width: 0;
display: block;
}
.add_rating_link_after {
border-radius: 1rem;
background-color: $ic-link-color;
text-align: center;
vertical-align: middle;
display: table-cell;
}
.add_rating_link i::before {
font-size: .75rem;
padding: .1875rem;
}
.add_rating_link i {
color: $ic-color-light;
}
tr.criterion td.criterion_description .description_content {
float: left;
clear: both;
}
}
.rubric_table {
border-collapse: collapse;
//for some reason, chrome puts a scrollbar on the table in the extended gradebook if it is 100%
width: 99.9%;
height: 100%;
page-break-inside: avoid;
}
thead th {
@ -70,7 +93,7 @@
}
td, th {
border: 1px solid $ic-border-color;
padding: 2px 5px;
padding: 7px 10px;
}
.rubric_title {
background-color: $ic-color-medium-light;
@ -78,7 +101,6 @@
border-left: 1px solid $ic-border-color;
border-right: 1px solid $ic-border-color;
padding: 5px;
margin-right: 1px;
font-weight: bold;
}
.has-assessments-warning {
@ -86,11 +108,19 @@
font-weight: normal;
}
tr.criterion td.criterion_description {
// :max-width 150px
height: 100%;
vertical-align: top;
.container {
@include revert_bootstrap_container;
position: relative;
font-size: 0.9em;
display: table;
height: 100%;
width: 100%;
}
.description_content {
display: table-cell;
vertical-align: middle;
}
.learning_outcome_flag, .threshold {
display: none;
@ -132,6 +162,7 @@
table.ratings {
border-collapse: collapse;
width: 100%;
height: 100%;
margin-left: -1px;
td {
border: 1px solid #aaa;
@ -141,11 +172,12 @@
border-top-width: 0;
font-size: 0.8em;
vertical-align: top;
padding: 2px 8px;
padding: 2px 10px;
.container {
@include revert_bootstrap_container;
padding: 5px 0;
position: relative;
height: 100%;
}
&.edge_rating .delete_rating_link {
display: none;
@ -223,6 +255,16 @@
td.editing, th.editing {
display: table-cell;
}
table.ratings td {
border-left: 1px dashed $ic-border-dark;
border-right: 1px dashed $ic-border-dark;
}
table.ratings td:nth-of-type(1) {
border-left: 0;
}
table.ratings td:nth-last-of-type(1) {
border-right: 0;
}
tr.criterion td.criterion_description {
.long_description_holder {
font-size: 0.8em;

View File

@ -137,7 +137,6 @@
<th scope="col"><%= t 'headers.criteria', "Criteria" %></th>
<th scope="col"><%= t 'headers.ratings', "Ratings" %></th>
<th scope="col"><%= t 'headers.points', "Pts" %></th>
<th scope="col" class="editing">&nbsp;</th>
</tr>
</thead>
<tbody>

View File

@ -31,32 +31,36 @@
description = criterion.try(:description) || t('defaults.description', "Description of criterion")
%>
<tr id="criterion_<%= criterion ? criterion.id : "blank" %>" class="criterion <%= "blank" unless criterion %> <%= 'ignore_criterion_for_scoring' if criterion && criterion.ignore_for_scoring %> <%= 'learning_outcome_criterion' if learning_outcome_criterion %>" style="<%= hidden unless criterion %>">
<td class="criterion_description hover-container">
<td class="criterion_description hover-container pad-box-micro">
<div class="container">
<i class="learning_outcome_flag icon-outcomes" aria-hidden="true"></i>
<span class="screenreader-only"><%= t('titles.linked_to_learning_outcome', "This criterion is linked to a Learning Outcome") %></span>
<span class="description criterion_description_value"><%= description %></span>
<span class="learning_outcome_id" style="display: none;"><%= criterion.try(:learning_outcome_id) %></span>
<span class="criterion_id" style="display: none;"><%= criterion.try(:id) %></span>
<div class="long_description_holder editing <%= 'empty' if !criterion || criterion.long_description.blank? %>">
<a href="#" class="long_description_link"><%= t 'links.view_longer_description', "view longer description" %></a>
<textarea class="long_description" aria-label="<%= t 'labels.long_description', "Long Description" %>" style="display: none;"><%= h((criterion.try(:long_description) || '')) %></textarea>
<div class="links editing">
<% if !learning_outcome_criterion %>
<a href="#" class="edit_criterion_link"><i class="icon-edit standalone-icon"></i><span class="screenreader-only"><%= t 'Edit criterion description' %></span></a>
<% end %>
<a href="#" class="delete_criterion_link"><i class="icon-trash standalone-icon"></i><span class="screenreader-only"><%= t 'Delete criterion row' %></span></a>
</div>
<div class="hide_when_learning_outcome <%= range_rating_visible ? '' : 'hidden' %>">
<div class="criterion_use_range_div editing">
<label><%= t 'Range' %>
<input type="checkbox" class="criterion_use_range" <%= 'checked' if (criterion.try(:criterion_use_range) && range_rating_enabled) %>/></label>
<div class="description_content">
<i class="learning_outcome_flag icon-outcomes" aria-hidden="true"></i>
<span class="screenreader-only"><%= t('titles.linked_to_learning_outcome', "This criterion is linked to a Learning Outcome") %></span>
<span class="description criterion_description_value"><%= description %></span>
<span class="learning_outcome_id" style="display: none;"><%= criterion.try(:learning_outcome_id) %></span>
<span class="criterion_id" style="display: none;"><%= criterion.try(:id) %></span>
<div class="long_description_holder editing <%= 'empty' if !criterion || criterion.long_description.blank? %>">
<a href="#" class="long_description_link"><%= t 'links.view_longer_description', "view longer description" %></a>
<textarea class="long_description" aria-label="<%= t 'labels.long_description', "Long Description" %>" style="display: none;"><%= h((criterion.try(:long_description) || '')) %></textarea>
</div>
<div class="hide_when_learning_outcome <%= range_rating_visible ? '' : 'hidden' %>">
<div class="criterion_use_range_div editing">
<label><%= t 'Range' %>
<input type="checkbox" class="criterion_use_range" <%= 'checked' if (criterion.try(:criterion_use_range) && range_rating_enabled) %>/></label>
</div>
</div>
<div class="threshold">
<%= before_label :threshold, "threshold" %>
<%= t :points, "%{points} pts", :points => %{<span class="mastery_points">#{n((criterion.mastery_points rescue 5))}</span>}.html_safe %>
</div>
</div>
<div class="threshold">
<%= before_label :threshold, "threshold" %>
<%= t :points, "%{points} pts", :points => %{<span class="mastery_points">#{n((criterion.mastery_points rescue 5))}</span>}.html_safe %>
</div>
<% if !learning_outcome_criterion %>
<div class="links editing">
<a href="#" class="edit_criterion_link"><i class="icon-edit standalone-icon"></i><span class="screenreader-only"><%= t 'icons.edit_description', 'Edit criterion description' %></span></a>
</div>
<% end %>
</div>
<% if rubric && !rubric.free_form_criterion_comments && assessment_rating && assessment_rating[:comments] %>
<div class="assessment-comments">
@ -98,6 +102,12 @@
%>
<td id="rating_<%= rating.id %>" class="rating <%= "edge_rating" if rating.edge %> <%= "infinitesimal" if rating_min.try(:to_f) == rating.points.try(:to_f) %>">
<div class="container">
<% if !learning_outcome_criterion %>
<div class="editing links">
<a href="#" class="edit_rating_link"><i class="icon-edit standalone-icon"></i><span class="screenreader-only"><%= t 'Edit rating' %></span></a>
<a href="#" class="delete_rating_link"><i class="icon-trash standalone-icon" ></i><span class="screenreader-only"><%= t 'Delete rating' %></span></a>
</div>
<% end %>
<div class="description rating_description_value"><%= rating.description %></div>
<span class="nobr">
<%= t "%{points}%{range_rating} pts",
@ -106,11 +116,8 @@
</span>
<span class="rating_id" style="display: none;"><%= rating && rating.id %></span>
<% if !learning_outcome_criterion %>
<div class="links">
<a href="#" class="edit_rating_link"><i class="icon-edit standalone-icon"></i><span class="screenreader-only"><%= t 'icons.edit', 'Edit rating' %></span></a>
<a href="#" class="delete_rating_link"><i class="icon-end standalone-icon"></i><span class="screenreader-only"><%= t 'icons.delete', 'Delete rating' %></span></a>
<!-- Hidden if last -->
<a href="#" class="add_rating_link_after"><i class="icon-add standalone-icon"></i><span class="screenreader-only"><%= t 'icons.add_new_rating_after', 'Add new rating category after current rating' %></span></a>
<div class="editing links add_rating_link">
<a href="#" class="add_rating_link_after" aria-label="<%= t 'Add rating' %>"><i class="icon-add icon-Solid"></i></a>
</div>
<% end %>
</div>
@ -169,13 +176,4 @@
<% end %>
</div>
</td>
<td class="editing">
<a
href="#"
class="delete_criterion_link"
title="<%= t(:delete_criterion_row, 'Delete Criterion Row') %>"
role="button"
aria-label="<%= t(:delete_criterion_row, 'Delete Criterion Row') %>"
><i class="icon-end"></i></a>
</td>
</tr>

View File

@ -134,7 +134,8 @@ import 'compiled/jquery/fixDialogButtons'
$criterion.find(".criterion_description").val(outcome.get('title')).focus().select();
$criterion.find(".mastery_points").text(outcome.get('mastery_points'));
$criterion.find(".links").remove();
$criterion.find(".edit_criterion_link").remove();
$criterion.find(".rating .links").remove();
},
hideCriterionAdd: function($rubric) {
$rubric.find('.add_right, .add_left, .add_column').removeClass('add_left add_right add_column');
@ -292,7 +293,7 @@ import 'compiled/jquery/fixDialogButtons'
var $ratingsContainers = $ratings.find('.rating .container').css('height', ""),
maxHeight = Math.max(
$ratings.height(),
$this.find(".criterion_description .container").height()
$this.find(".criterion_description .container .description_content").height()
);
// the -10 here is the padding on the .container.
$ratingsContainers.css('height', (maxHeight - 10) + 'px');
@ -481,13 +482,14 @@ import 'compiled/jquery/fixDialogButtons'
rating.min_points = rubricEditing.localizedPoints(criterion.ratings[count].points)
}
var $rating = $rating_template.clone(true);
$rating.toggleClass('edge_rating', count === 0 || count === criterion.ratings.length - 1);
$rating.toggleClass('edge_rating', count === 1 || count === criterion.ratings.length);
$rating.fillTemplateData({data: rating});
$rating.find('.range_rating').showIf(criterion.criterion_use_range === true && rating.min_points !== rating.points);
$criterion.find(".ratings").append($rating);
});
if (criterion.learning_outcome_id) {
$criterion.find(".links").remove();
$criterion.find(".edit_criterion_link").remove();
$criterion.find(".rating .links").remove();
}
$rubric.find(".summary").before($criterion);
$criterion.find(".criterion_points").val(criterion.points).blur();
@ -922,64 +924,6 @@ import 'compiled/jquery/fixDialogButtons'
if(!$target.closest('.ratings').length) {
rubricEditing.hideCriterionAdd($target.parents('.rubric'));
}
}).delegate('.rating', 'mousemove', function(event) {
var $this = $(this),
$rubric = $this.parents(".rubric");
if($rubric.find(".rating.editing").length > 0 || $this.parents(".criterion").hasClass('learning_outcome_criterion')) {
rubricEditing.hideCriterionAdd($rubric);
return false;
}
var expandPadding = 10;
if(!$.data(this, 'hover_offset')) {
$.data(this, 'hover_offset', $this.offset());
$.data(this, 'hover_width', $this.outerWidth());
var points = $.data(this, 'points', numberHelper.parse($this.find(".points").text()));
var prevPoints = $.data(this, 'prev_points', numberHelper.parse($this.prev(".rating").find(".points").text()));
var nextPoints = $.data(this, 'next_points', numberHelper.parse($this.next(".rating").find(".points").text()));
$.data(this, 'prev_diff', Math.abs(points - prevPoints));
$.data(this, 'next_diff', Math.abs(points - nextPoints));
}
var offset = $.data(this, 'hover_offset');
var width = $.data(this, 'hover_width');
var $ratings = $this.parents(".ratings");
var x = event.pageX;
var y = event.pageY;
var leftSide = false;
if(x <= offset.left + (width / 2)) {
leftSide = true;
}
var $lastHover = $ratings.data('hover_rating');
var lastLeftSide = $ratings.data('hover_left_side');
if(!$lastHover || $this[0] != $lastHover[0] || leftSide != lastLeftSide) {
rubricEditing.hideCriterionAdd($rubric);
var $prevRating, $nextRating;
if(leftSide && ($prevRating = $this.prev(".rating")) && $prevRating.length) {// && $(this).data('prev_diff') > 1) {
$this.addClass('add_left');
$prevRating.addClass('add_right');
$this[(x <= offset.left + expandPadding) ? 'addClass': 'removeClass']('add_column');
} else if(!leftSide && ($nextRating = $this.next(".rating")) && $nextRating.length) {// && $(this).data('next_diff') > 1) {
$this.addClass('add_right');
$nextRating.addClass('add_left');
$this[(x >= offset.left + width - expandPadding) ? 'addClass' : 'removeClass']('add_column');
}
} else if($lastHover) {
if(leftSide) {
if(x <= offset.left + expandPadding && $.data(this, 'prev_diff') > 1) {
$this.addClass('add_column');
} else {
$this.removeClass('add_column');
}
} else {
if(x >= offset.left + width - expandPadding && $.data(this, 'next_diff') > 1) {
$this.addClass('add_column');
} else {
$this.removeClass('add_column');
}
}
}
return false;
}).delegate('.rating', 'mouseout', function(event) {
$(this).data('hover_offset', null).data('hover_width', null);
}).delegate('.delete_rating_link', 'click', function(event) {
const $rating_cell = $(this).closest('td')
const $target = $rating_cell.next().find('.edit_rating_link');

View File

@ -32,7 +32,7 @@ describe "rubrics" do
expect(response).to be_success
page = Nokogiri::HTML(response.body)
expect(page.css('#rubrics .rubric_table .criterion:nth-child(1) .links')).to be_empty
expect(page.css('#rubrics .rubric_table .criterion:nth-child(2) .links')).not_to be_empty
expect(page.css('#rubrics .rubric_table .criterion:nth-child(1) .edit_criterion_link')).to be_empty
expect(page.css('#rubrics .rubric_table .criterion:nth-child(2) .edit_criterion_link')).not_to be_empty
end
end