Skip to content

Commit b21922a

Browse files
committed
Feature: Replace lesson View Course button with Previous Lesson
Because: With the new course-contents sidebar on the lesson page providing a "Back to course" link on both mobile and desktop, the View Course button at the bottom of the lesson was redundant. Swapping it for a Previous Lesson button gives users a natural pair with the existing Next Lesson button for sequential navigation through a course. This PR: - Adds Course#previous_lesson and refactors Course#next_lesson to share the same range-based query style - Replaces the "View Course" button in app/views/lessons/_lesson_buttons.html.erb with a conditional Previous Lesson button that is hidden on the first lesson - Reworks the lesson button row from `flex justify-between` to a 3-column grid on md+ so Complete stays centered and Previous/Next stay anchored to the row edges regardless of which neighbours render; mobile keeps the existing full-width stacked layout via `display: contents` on the slot wrappers
1 parent 1216aba commit b21922a

4 files changed

Lines changed: 100 additions & 35 deletions

File tree

app/models/course.rb

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,10 @@ def progress_for(user)
1818
end
1919

2020
def next_lesson(lesson)
21-
lessons.where('lessons.position > ?', lesson.position).first
21+
lessons.where(lessons: { position: lesson.position.next.. }).order(:position).first
22+
end
23+
24+
def previous_lesson(lesson)
25+
lessons.where(lessons: { position: ...lesson.position }).order(position: :desc).first
2226
end
2327
end
Lines changed: 36 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1,29 +1,43 @@
1-
<div class="flex flex-col justify-between max-w-sm mx-auto space-y-6 md:max-w-full md:space-y-0 md:flex-row md:space-x-7">
2-
<%= link_to path_course_path(course.path, course), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'view-course-btn' } do %>
3-
<%= inline_svg_tag 'icons/academic-cap.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'View course', desc: 'Go to course page' %>
4-
View Course
5-
<% end %>
6-
7-
<% if user_signed_in? %>
8-
<%= render Complete::ButtonComponent.new(lesson:) %>
1+
<div class="flex flex-col gap-y-6 max-w-sm mx-auto md:grid md:grid-cols-3 md:gap-x-7 md:gap-y-0 md:max-w-full md:items-center">
2+
<% if course.previous_lesson(lesson).present? %>
3+
<div class="contents md:block md:justify-self-start">
4+
<%= link_to lesson_path(course.previous_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'previous-lesson-btn' } do %>
5+
<%= inline_svg_tag 'icons/arrow-left-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Previous lesson', desc: 'Go to previous lesson' %>
6+
Previous Lesson
7+
<% end %>
8+
</div>
99
<% else %>
10-
<%= link_to(
11-
'Sign in to track progress',
12-
sign_in_path,
13-
class: 'button button--primary',
14-
data: { test_id: 'sign_in_button' }
15-
) %>
10+
<div class="hidden md:block" aria-hidden="true"></div>
1611
<% end %>
1712

18-
<% if course.next_lesson(lesson).present? %>
19-
<%= link_to lesson_path(course.next_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'next-lesson-btn' } do %>
20-
<%= inline_svg_tag 'icons/arrow-right-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Next lesson', desc: 'Go to next lesson' %>
21-
Next Lesson
13+
<div class="contents md:block md:justify-self-center">
14+
<% if user_signed_in? %>
15+
<%= render Complete::ButtonComponent.new(lesson:) %>
16+
<% else %>
17+
<%= link_to(
18+
'Sign in to track progress',
19+
sign_in_path,
20+
class: 'button button--primary',
21+
data: { test_id: 'sign_in_button' }
22+
) %>
2223
<% end %>
24+
</div>
25+
26+
<% if course.next_lesson(lesson).present? %>
27+
<div class="contents md:block md:justify-self-end">
28+
<%= link_to lesson_path(course.next_lesson(lesson)), class: 'button button--secondary px-4 py-2 font-medium', data: { test_id: 'next-lesson-btn' } do %>
29+
<%= inline_svg_tag 'icons/arrow-right-circle.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Next lesson', desc: 'Go to next lesson' %>
30+
Next Lesson
31+
<% end %>
32+
</div>
2333
<% elsif lesson.choose_path_lesson? %>
24-
<%= link_to paths_url, class: 'button button--secondary px-4 font-medium', data: { test_id: 'choose-path-lesson-btn' } do %>
25-
<%= inline_svg_tag 'icons/map.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Choose path', desc: 'Go to paths page' %>
26-
Choose Path
27-
<% end %>
34+
<div class="contents md:block md:justify-self-end">
35+
<%= link_to paths_url, class: 'button button--secondary px-4 font-medium', data: { test_id: 'choose-path-lesson-btn' } do %>
36+
<%= inline_svg_tag 'icons/map.svg', class: 'h-8 w-8 pr-2', aria: true, title: 'Choose path', desc: 'Go to paths page' %>
37+
Choose Path
38+
<% end %>
39+
</div>
40+
<% else %>
41+
<div class="hidden md:block" aria-hidden="true"></div>
2842
<% end %>
2943
</div>

spec/models/course_spec.rb

Lines changed: 30 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -37,20 +37,45 @@
3737
it 'returns the next lesson' do
3838
course = create(:course)
3939
section = create(:section, course:)
40-
lesson_one = create(:lesson, position: 1, section:)
41-
lesson_two = create(:lesson, position: 2, section:)
40+
next_lesson = create(:lesson, position: 3, section:)
41+
current_lesson = create(:lesson, position: 2, section:)
42+
_previous_lesson = create(:lesson, position: 1, section:)
4243

43-
expect(course.next_lesson(lesson_one)).to eql(lesson_two)
44+
expect(course.next_lesson(current_lesson)).to eql(next_lesson)
4445
end
4546
end
4647

4748
context 'when there is no next lesson' do
4849
it 'returns nothing' do
4950
course = create(:course)
5051
section = create(:section, course:)
51-
lesson_one = create(:lesson, position: 1, section:)
52+
current_lesson = create(:lesson, position: 1, section:)
5253

53-
expect(course.next_lesson(lesson_one)).to be_nil
54+
expect(course.next_lesson(current_lesson)).to be_nil
55+
end
56+
end
57+
end
58+
59+
describe '#previous_lesson' do
60+
context 'when there is a previous lesson' do
61+
it 'returns the previous lesson' do
62+
course = create(:course)
63+
section = create(:section, course:)
64+
current_lesson = create(:lesson, position: 2, section:)
65+
previous_lesson = create(:lesson, position: 1, section:)
66+
_next_lesson = create(:lesson, position: 3, section:)
67+
68+
expect(course.previous_lesson(current_lesson)).to eql(previous_lesson)
69+
end
70+
end
71+
72+
context 'when there is no previous lesson' do
73+
it 'returns nothing' do
74+
course = create(:course)
75+
section = create(:section, course:)
76+
current_lesson = create(:lesson, position: 1, section:)
77+
78+
expect(course.previous_lesson(current_lesson)).to be_nil
5479
end
5580
end
5681
end

spec/system/lesson_navigation_spec.rb

Lines changed: 29 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -44,15 +44,37 @@
4444
end
4545
end
4646

47-
describe 'the View Course button' do
48-
it 'directs to the course view' do
49-
visit lesson_path(lesson)
50-
find(:test_id, 'view-course-btn').click
47+
describe 'the previous lesson button' do
48+
context 'when the previous lesson is within the same section' do
49+
let!(:current_lesson) { create(:lesson, position: 2, section:) }
5150

52-
expect(find(:test_id, 'course-title-header')).to have_text(/#{course.title}/i)
51+
it 'moves to the previous lesson in the section when clicked' do
52+
visit lesson_path(current_lesson)
53+
find(:test_id, 'previous-lesson-btn').click
5354

54-
within '[data-test-id="lesson-list"]', match: :first do
55-
expect(page).to have_text(/#{lesson.title}/i)
55+
expect(find(:test_id, 'lesson-title-header')).to have_text(/#{lesson.title}/i)
56+
end
57+
end
58+
59+
context 'when on the first lesson of a section' do
60+
let!(:next_section) { create(:section, position: 2, course:) }
61+
let!(:next_section_lesson) { create(:lesson, position: 2, section: next_section) }
62+
63+
it 'moves to the last lesson in the previous section when clicked' do
64+
visit lesson_path(next_section_lesson)
65+
find(:test_id, 'previous-lesson-btn').click
66+
67+
expect(find(:test_id, 'lesson-title-header')).to have_text(/#{lesson.title}/i)
68+
end
69+
end
70+
71+
context 'when on the first lesson in the course' do
72+
it 'is not present' do
73+
visit lesson_path(lesson)
74+
75+
expect(page).to have_css('[data-test-id]')
76+
77+
expect(page).to have_no_css('[data-test-id="previous-lesson-btn"]')
5678
end
5779
end
5880
end

0 commit comments

Comments
 (0)