Skip to content

Commit

Permalink
remove obsolete course sidebar look and feel
Browse files Browse the repository at this point in the history
closes ADMIN-2778
flag=none

test plan:
 - confirm 'Accessible Course Menu' feature flag is gone
 - course sidebar menu maintains all recent a11y changes:
   * aria-current="page" on current page
   * uses course color
   * high contrast mode works right
   * basically, compare it to production behavior

Change-Id: Iae20776effa9421ed26c963f6d148ed0b118fab3
Reviewed-on: https://gerrit.instructure.com/c/canvas-lms/+/214631
Tested-by: Jenkins
Reviewed-by: Rex Fleischer <[email protected]>
Reviewed-by: Mysti Lilla <[email protected]>
QA-Review: Mysti Lilla <[email protected]>
Product-Review: Carl Kibler <[email protected]>
  • Loading branch information
Carl Kibler committed Oct 31, 2019
1 parent 0a09b08 commit 7ae4d37
Show file tree
Hide file tree
Showing 11 changed files with 8 additions and 358 deletions.
66 changes: 1 addition & 65 deletions app/helpers/section_tab_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -73,8 +73,7 @@ def section_tabs
end

def section_tab_tag(tab, context, active_tab)
tab_generator = @domain_root_account.try(:feature_enabled?, :a11y_left_menu) ? SectionTabTagNew : SectionTabTag
concat(tab_generator.new(tab, context, active_tab).to_html)
concat(SectionTabTag.new(tab, context, active_tab).to_html)
end

class AvailableSectionTabs
Expand Down Expand Up @@ -146,69 +145,6 @@ def a_classes
end
end

def a_aria_current_page
'page' if @tab.active?(@active_tab)
end

def a_attributes
{ href: @tab.path,
title: @tab.label,
'aria-current': a_aria_current_page,
class: a_classes }.tap do |h|
h[:target] = @tab.target if @tab.target?
end
end

def a_tag
content_tag(:a, a_attributes) do
concat(@tab.label)
concat(span_tag)
end
end

def li_classes
[ 'section' ].tap do |a|
a << 'section-tab-hidden' if @tab.hide? || @tab.unused?
end
end

def span_tag
if @tab.hide? || @tab.unused?
if @tab.hide?
text = I18n.t('* Disabled in Course Settings')
else
text = I18n.t('* No content has been added')
end
content_tag(:span, text, {
id: 'inactive_nav_link',
class: 'screenreader-only'
})
end
end

def to_html
content_tag(:li, a_tag, {
class: li_classes
})
end
end

class SectionTabTagNew
include ActionView::Context
include ActionView::Helpers::TagHelper
include ActionView::Helpers::TextHelper

def initialize(tab, context, active_tab=nil)
@tab = SectionTabPresenter.new(tab, context)
@active_tab = active_tab
end

def a_classes
[ @tab.css_class.downcase.replace_whitespace('-') ].tap do |a|
a << 'active' if @tab.active?(@active_tab)
end
end

def a_title
if @tab.hide?
I18n.t('Disabled. Not visible to students')
Expand Down
10 changes: 0 additions & 10 deletions app/stylesheets/base/_#left-side.scss
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,3 @@
#left-side {
display: none;
}

#section-tabs .section-tab-hidden a {
@if $use_high_contrast {
color: darken($ic-course-sidenav_list-item--inactive-font-color, 45%);
background: lighten($ic-course-sidenav_list-item--inactive-font-color, 15%);
}
@else {
color: $ic-course-sidenav_list-item--inactive-font-color;
}
}
87 changes: 0 additions & 87 deletions app/stylesheets/components/_components.scss
Original file line number Diff line number Diff line change
Expand Up @@ -24,10 +24,6 @@
}

// List View
/* .list-view-a11y-left-menu-flag supersedes this based on a11y_left_menu
release flag. After release, ADMIN-2778 will switch back to .list-view
name with new CSS.
*/
.list-view {
overflow: auto;
& > header,
Expand All @@ -51,89 +47,6 @@
border-top: 0 !important; // oldskool important
}

& > ul,
// oldskool overrides
& > nav > ul {
list-style: none;
margin: 0;
padding: 0;

& > li {
padding: 0;
& > a {
display: block;
text-decoration: none;
color: $ic-font-color-dark;
border-radius: $baseBorderRadius;
padding: ($ic-sp - 4) $ic-sp;
overflow-wrap: break-word;
word-wrap: break-word;
hyphens: auto;
line-height: $ic-label-line-height;

@if $use_high_contrast {
text-decoration: underline;
}

&:hover, &:focus {
background: lighten($ic-color-neutral, 5%);
}

& > span {
white-space: nowrap;
}
}

&:last-child > a {
border-bottom: 0;
}
}
}

// lists w/o headers still need a strong border
& > ul + ul {
border-top: 1px solid $ic-border-light;
}

a.active {
background-color: var(--ic-brand-primary);
font-weight: bold;
color: $ic-color-light;

// offset focus ring so it doesn't blend in with background
&:focus { @include ic-focus-variant($offset: 0.0625rem); }

&:hover, &:focus { background-color: var(--ic-brand-primary); }
.nav-badge {
background: $ic-color-light;
color: var(--ic-brand-primary);
}
}
}

.list-view-a11y-left-menu-flag {
overflow: auto;
& > header,
// oldskool compat
#section-tabs-header {
@include fontSize($ic-font-size--small);
font-weight: bold;
margin: 0 0 $ic-sp/2;
white-space: nowrap;
overflow: hidden;
text-overflow: ellipsis;
}
#section-tabs-header-subtitle {
display: block;
@include fontSize(11px);
font-style: italic;
color: inherit;
}

& > *:first-child {
border-top: 0 !important; // oldskool important
}

& > nav.theme-preview {
padding: 3px;
}
Expand Down
2 changes: 1 addition & 1 deletion app/views/brand_configs/show.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,7 @@
<div class="content-box">
<div class="grid-row">
<div class="col-xs-3">
<div class="<%= @domain_root_account.feature_enabled?(:a11y_left_menu) ? 'list-view-a11y-left-menu-flag' : 'list-view' %>">
<div class="list-view">
<nav class="theme-preview" aria-label="context" role="navigation">
<ul>
<li><a href="#"><%= t('Context nav link') %></a></li>
Expand Down
2 changes: 0 additions & 2 deletions app/views/layouts/application.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -119,8 +119,6 @@
<% if @show_left_side %>
<% if @no_left_side_list_view
list_view_class = ''
elsif @domain_root_account.try(:feature_enabled?, :a11y_left_menu)
list_view_class = 'list-view-a11y-left-menu-flag'
else
list_view_class = 'list-view'
end
Expand Down
7 changes: 0 additions & 7 deletions config/feature_flags/capsaicin_release_flags.yml
Original file line number Diff line number Diff line change
@@ -1,11 +1,4 @@
---
a11y_left_menu:
state: hidden
display_name: Accessible Sidebar Menu
description: Improve accessibility of left-hand menu through better
use of color plus visual and screenreader indicators of inactive menu
items.
applies_to: RootAccount
check_submission_file_type:
state: hidden
applies_to: RootAccount
Expand Down
Loading

0 comments on commit 7ae4d37

Please sign in to comment.