Feature: Add course contents sidebar to the lesson page#5311
Conversation
KevinMulhern
force-pushed
the
feature/lesson-sidebar
branch
2 times, most recently
from
64b9b20 to
a1cf7ca
Compare
| @@ -0,0 +1,90 @@ | |||
| <%# Off-canvas drawer — below xl %> | |||
There was a problem hiding this comment.
Since this is the third place we use this pattern (we use it on the mobile nav and the admin sidebar as well), I've opened a PR to refactor it into a reusable component: #5312
There was a problem hiding this comment.
Pull request overview
This PR adds a course-contents sidebar to lesson pages so learners can navigate within a course, see progress/completion state, and keep the toggle available while reading. It fits into the lesson experience by moving course context/navigation out of the old header and into a lazy-loaded sidebar with Turbo/Stimulus updates.
Changes:
- Adds a new lesson course-contents endpoint plus sidebar components for desktop and mobile/off-canvas rendering.
- Reworks the lesson page layout/header to include a sticky sub-navbar, lazy sidebar frame, and live progress/completion updates.
- Adds supporting Stimulus/CSS changes and new request/component/system specs around sidebar behavior.
Reviewed changes
Copilot reviewed 28 out of 29 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
spec/system/lessons/sidebar_spec.rb |
System coverage for expanded-section persistence while navigating lessons. |
spec/requests/lessons/course_contents_spec.rb |
Request coverage for the new sidebar endpoint and completion state. |
spec/components/lessons/sidebar_component_spec.rb |
Component coverage for sidebar shells, sections, and progress bar rendering. |
spec/components/lessons/sidebar/section_component_spec.rb |
Section component coverage for titles, lesson lists, and default expansion. |
spec/components/lessons/sidebar/lesson_component_spec.rb |
Lesson row coverage for links, current-state, and completion icon rendering. |
spec/components/lessons/sidebar/completion_icon_component_spec.rb |
Completion icon coverage for completed/incomplete variants. |
config/routes.rb |
Adds nested course_contents route under lessons. |
app/views/lessons/show.html.erb |
Integrates the lazy-loaded sidebar, sticky sub-navbar, and updated lesson layout. |
app/views/lessons/course_contents/show.html.erb |
Renders the sidebar inside its Turbo Frame response. |
app/views/lessons/completions/create.turbo_stream.erb |
Updates sidebar completion icons and progress bar via Turbo Streams. |
app/views/lessons/_sub_navbar.html.erb |
Adds the sticky sub-navbar and sidebar toggle button. |
app/views/lessons/_sidebar_skeleton.html.erb |
Adds loading skeleton markup for the lazy sidebar frame. |
app/views/lessons/_header.html.erb |
Simplifies the lesson header to focus on the lesson title. |
app/javascript/controllers/visibility_controller.js |
Adjusts visibility controller initialization behavior. |
app/javascript/controllers/sticky_state_controller.js |
Adds sticky-state detection for styling the sub-navbar when pinned. |
app/controllers/lessons/course_contents_controller.rb |
Loads lesson/course/sections data for the sidebar frame endpoint. |
app/components/lessons/sidebar_toggle_controller.js |
Adds sidebar toggle, persistence, and current-link sync behavior. |
app/components/lessons/sidebar_component.rb |
Provides sidebar component state, including progress calculation. |
app/components/lessons/sidebar_component.html.erb |
Renders mobile drawer and desktop sidebar markup. |
app/components/lessons/sidebar/section_component.rb |
Encapsulates section expansion logic. |
app/components/lessons/sidebar/section_component.html.erb |
Renders collapsible course sections and nested lesson rows. |
app/components/lessons/sidebar/progress_bar_component.rb |
Wraps sidebar progress-bar state. |
app/components/lessons/sidebar/progress_bar_component.html.erb |
Renders the sidebar progress bar UI. |
app/components/lessons/sidebar/lesson_component.rb |
Encapsulates lesson-row state and icon selection. |
app/components/lessons/sidebar/lesson_component.html.erb |
Renders lesson links and optional completion indicators. |
app/components/lessons/sidebar/completion_icon_component.rb |
Encapsulates completion icon styling/title logic. |
app/components/lessons/sidebar/completion_icon_component.html.erb |
Renders the per-lesson completion SVG icon. |
app/assets/stylesheets/custom_styles/layout.css |
Adds global scroll padding and reusable thin-scrollbar utility. |
app/assets/images/icons/panel-left.svg |
Adds the new sidebar-toggle icon asset. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
KevinMulhern
force-pushed
the
feature/lesson-sidebar
branch
from
a1cf7ca to
36de006
Compare
|
Would this also close #4423 or be within scope to address it too? |
|
Theres been a lot more activity on that one since last I looked at it! lol This one won't close it sadly @mao-sz, and I think it falls just outside of the scope of this PR. But I think the sticky sub-navbar introduced in this could provide some good options for handling the lesson toc on mobile. I catch up on the latest on that issue, and drop some thoughts on it. |
|
Makes sense. focus order between course-toc/lesson/lesosn-toc is finicky. |
zachmmeyer
left a comment
zachmmeyer
left a comment
There was a problem hiding this comment.
Gave it a good twice over.
Percentages add up correctly, maintains persistence across courses and with logging in and out (across browsers too), and it looks great!
Fantastic work!
✅
github-project-automation
Bot
moved this from 📋 Backlog / Ideas
to 👀 In review
in Main Site
mao-sz
left a comment
mao-sz
left a comment
There was a problem hiding this comment.
How would this play with #5199? The concern I have is that wrapping the application yield with <main id="main-content"> won't be sufficient now that we have the sidebar within it and above the lesson content in the hierarchy.
KevinMulhern
force-pushed
the
feature/lesson-sidebar
branch
from
36de006 to
1eaf3ab
Compare
🤔 this is a good point. I think you're right @mao-sz, we might need to change the approach for that issue to something more dynamic for different layouts. Maybe something like if an inner main is present in the view like on lesson pages use that, otherwise fallback to the main around yield. |
|
I'll leave that for you to communicate in that issue then @KevinMulhern, since you'll have a better idea of how to incorporate that with this. I should have some more time tomorrow for a more thorough playaround, but with the sidebar lesson link change, it certainly feels nicer to interact with now. |
| @apply max-w-7xl mx-auto py-10 px-4 sm:py-14 sm:px-6 lg:px-8; | ||
| } | ||
|
|
||
| html:has([data-controller~="lessons--sidebar-toggle"]) { |
There was a problem hiding this comment.
Is the ~= intentional here or supposed to be =?
There was a problem hiding this comment.
Good eye! this is a future proofing thing. Having = here would work and be ok, but it could be brittle. The data-controller attribute can take multiple space-separated controllers like:
data-controller="lessons--sidebar-toggle toc pin").
If we were to add another controller to this element in the future it would stop matching with =, whereas ~= will keep on matching no matter how many controllers we have attached.
| attr_reader :course, :sections, :current_lesson, :current_user | ||
|
|
||
| def progress_percentage | ||
| return nil unless current_user |
There was a problem hiding this comment.
No issue here, just always get a chuckle with return ... unless ... as if it's "EVERYONE PANIC! Unless everything goes as we expect.", courtesy of Mark Rendle.
There was a problem hiding this comment.
lool that was a good watch!
Thanks for pointing this one out. I'll change this to an if instead. I've been preferring straight if conditions over unless these days because they're generally way more readable.
## Because The lesson page had no in-page way to navigate between lessons in the same course — learners had to back out to the course page to switch lessons, and the existing header repeated course context (badge, course title link) on every lesson. Adding a persistent course-contents sidebar lets users see where they are in the course, jump between lessons, and track completion without leaving the lesson. ## This PR - Adds a collapsible left sidebar to the lesson page listing every section and lesson in the current course, with the active lesson highlighted and its section expanded by default - Introduces `Lessons::SidebarComponent`, `Lessons::Sidebar::SectionComponent`, and `Lessons::Sidebar::LessonComponent`, lazy-loaded via a Turbo Frame backed by a new `Lessons::CourseContentsController` - Shows per-lesson completion icons and an overall course progress bar in the sidebar; both update in place when a lesson is completed via Turbo Streams - Adds a sticky lesson sub-navbar with a toggle button; the collapsed state persists in `localStorage` via a new `lessons--sidebar-toggle` Stimulus controller - Simplifies the lesson header — removes the course badge and course title link, leaving the lesson title
KevinMulhern
force-pushed
the
feature/lesson-sidebar
branch
from
1eaf3ab to
1b836db
Compare
|
Thanks for the great feedback @mao-sz. This is ready for another look over when you have some time please :) |
Because
The lesson page had no in-page way to navigate between lessons in the same course - learners had to back out to the course page to switch lessons, and the existing header repeated course context (badge, course title link) on every lesson. Adding a persistent course-contents sidebar lets users see where they are in the course, jump between lessons, and track completion without leaving the lesson.
Closes: #3045
This PR
Lessons::SidebarComponent,Lessons::Sidebar::SectionComponent, andLessons::Sidebar::LessonComponent, lazy-loaded via a Turbo Frame backed by a newLessons::CourseContentsControllerlocalStoragevia a newlessons--sidebar-toggleStimulus controller