Feature: Replace lesson View Course button with Previous Lesson#5328
Open
KevinMulhern wants to merge 1 commit into
Open
Feature: Replace lesson View Course button with Previous Lesson#5328KevinMulhern wants to merge 1 commit into
KevinMulhern wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR replaces the redundant “View Course” button at the bottom of lesson pages with a “Previous Lesson” button to improve sequential navigation (pairing with the existing “Next Lesson” button), supported by new Course#previous_lesson behavior and related UI/test updates.
Changes:
- Add
Course#previous_lessonand refactorCourse#next_lessonto use range-based queries. - Update lesson button UI to conditionally render Previous/Next and switch md+ layout to a 3-column grid while preserving the mobile stacked layout.
- Update system specs to validate Previous Lesson navigation behavior and add model specs for
previous_lesson.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
app/models/course.rb |
Introduces previous_lesson and refactors next_lesson query approach. |
app/views/lessons/_lesson_buttons.html.erb |
Replaces “View Course” with conditional “Previous Lesson” and updates responsive layout. |
spec/system/lesson_navigation_spec.rb |
Replaces View Course system test with Previous Lesson navigation coverage. |
spec/models/course_spec.rb |
Adds unit coverage for Course#previous_lesson. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
KevinMulhern
force-pushed
the
feature/previous-lesson-button
branch
from
d8ebbe7 to
79e0a75
Compare
KevinMulhern
force-pushed
the
feature/previous-lesson-button
branch
from
79e0a75 to
e206061
Compare
KevinMulhern
force-pushed
the
feature/previous-lesson-button
branch
from
e206061 to
b21922a
Compare
Comment on lines
+1
to
+4
| <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"> | ||
| <% if course.previous_lesson(lesson).present? %> | ||
| <div class="contents md:block md:justify-self-start"> | ||
| <%= 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 %> |
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
KevinMulhern
force-pushed
the
feature/previous-lesson-button
branch
from
b21922a to
aa87177
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.
Closes: #2623
This PR:
Course#previous_lessonand refactorsCourse#next_lessonto share the same range-based query styleapp/views/lessons/_lesson_buttons.html.erbwith a conditional Previous Lesson button that is hidden on the first lessonflex justify-betweento 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 viadisplay: contentson the slot wrappers