Skip to content

Commit

Permalink
unify SR course nav labels
Browse files Browse the repository at this point in the history
This patchset also removes screenreader as an option for
SectionTabPresenter as it's no longer used anywhere.

closes: CNVS-37921

test plan:
Ensure the labels for each nav element match the visual label.
Specifically the following elements:
 - Announcements
 - Assignments
 - Discussions
 - Grades
 - Files
 - Settings

Change-Id: I8daec510a45dbfa399d9e3964c30be8c86a98fdb
Reviewed-on: https://gerrit.instructure.com/118569
Reviewed-by: Sheldon Leibole <[email protected]>
Reviewed-by: Shahbaz Javeed <[email protected]>
Tested-by: Jenkins
QA-Review: Anju Reddy <[email protected]>
Product-Review: Keith T. Garner <[email protected]>
  • Loading branch information
djbender committed Jul 12, 2017
1 parent 981364a commit 639e389
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 45 deletions.
1 change: 0 additions & 1 deletion app/helpers/section_tab_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,6 @@ def a_attributes
{ href: @tab.path,
title: @tab.label,
class: a_classes }.tap do |h|
h[:'aria-label'] = @tab.screenreader if @tab.screenreader?
h[:target] = @tab.target if @tab.target?
end
end
Expand Down
10 changes: 2 additions & 8 deletions app/models/course.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2417,28 +2417,24 @@ def self.default_tabs
:label => t('#tabs.announcements', "Announcements"),
:css_class => 'announcements',
:href => :course_announcements_path,
:screenreader => t("Course Announcements"),
:icon => 'icon-announcement'
}, {
:id => TAB_ASSIGNMENTS,
:label => t('#tabs.assignments', "Assignments"),
:css_class => 'assignments',
:href => :course_assignments_path,
:screenreader => t('#tabs.course_assignments', "Course Assignments"),
:icon => 'icon-assignment'
}, {
:id => TAB_DISCUSSIONS,
:label => t('#tabs.discussions', "Discussions"),
:css_class => 'discussions',
:href => :course_discussion_topics_path,
:screenreader => t("Course Discussions"),
:icon => 'icon-discussion'
}, {
:id => TAB_GRADES,
:label => t('#tabs.grades', "Grades"),
:css_class => 'grades',
:href => :course_grades_path,
:screenreader => t('#tabs.course_grades', "Course Grades")
:href => :course_grades_path
}, {
:id => TAB_PEOPLE,
:label => t('#tabs.people', "People"),
Expand All @@ -2454,7 +2450,6 @@ def self.default_tabs
:label => t('#tabs.files', "Files"),
:css_class => 'files',
:href => :course_files_path,
:screenreader => t("Course Files"),
:icon => 'icon-folder'
}, {
:id => TAB_SYLLABUS,
Expand Down Expand Up @@ -2495,8 +2490,7 @@ def self.default_tabs
:id => TAB_SETTINGS,
:label => t('#tabs.settings', "Settings"),
:css_class => 'settings',
:href => :course_settings_path,
:screenreader => t('#tabs.course_settings', "Course Settings")
:href => :course_settings_path
}
]
end
Expand Down
10 changes: 2 additions & 8 deletions app/presenters/section_tab_presenter.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,16 +23,12 @@ def initialize(tab, context)
@context = context
end
attr_reader :tab, :context
delegate :css_class, :label, :screenreader, :target, to: :tab
delegate :css_class, :label, :target, to: :tab

def active?(active_tab)
active_tab == tab.css_class
end

def screenreader?
tab.respond_to?(:screenreader)
end

def hide?
tab.hidden || tab.hidden_unused
end
Expand All @@ -50,8 +46,6 @@ def path_args
end

def to_h
{ css_class: tab.css_class, icon: tab.icon, hidden: hide?, path: path }.tap do |h|
h[:screenreader] = tab.screenreader if screenreader?
end
{ css_class: tab.css_class, icon: tab.icon, hidden: hide?, path: path }
end
end
9 changes: 0 additions & 9 deletions spec/helpers/section_tab_helper_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -169,15 +169,6 @@ class SectionTabHelperSpec
)

expect(tag.a_attributes.keys).to include(:href, :class)
expect(tag.a_attributes.keys).to_not include(:'aria-label')
end

it 'should include key aria-label if tab has screenreader text' do
tag = SectionTabHelperSpec::SectionTabTag.new(
tab_assignments, course
)

expect(tag.a_attributes.keys).to include(:'aria-label')
end

it 'includes a target if tab has the target attribute' do
Expand Down
19 changes: 0 additions & 19 deletions spec/presenters/section_tab_presenter_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,19 +43,6 @@
end
end

describe '#screenreader?' do
it 'should return false if tab has no screenreader element' do
expect(presenter.screenreader?).to be_falsey
end

it 'should return true when tab has screenreader element' do
new_presenter = SectionTabPresenter.new(
assignments_tab, course
)
expect(new_presenter.screenreader?).to be_truthy
end
end

describe '#target?' do
it 'returns true if the tab has a target attribute' do
expect(SectionTabPresenter.new(tab.merge(target: '_blank'), course).target?).to eq true
Expand Down Expand Up @@ -124,12 +111,6 @@
icon: 'icon-home'
}), course).to_h
expect(h.keys).to include(:icon, :hidden, :path)
expect(h).to_not have_key(:screenreader)
end

it 'should include screenreader text if present' do
h = SectionTabPresenter.new(assignments_tab, course).to_h
expect(h).to have_key(:screenreader)
end
end
end

0 comments on commit 639e389

Please sign in to comment.