feat(AU-2375): Update breadcrumbs and outline navigation bar UI #1582
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #1582 +/- ##
=======================================
Coverage 84.60% 84.60%
=======================================
Files 331 331
Lines 5656 5658 +2
Branches 1357 1359 +2
=======================================
+ Hits 4785 4787 +2
Misses 854 854
Partials 17 17 ☔ View full report in Codecov by Sentry. |
|
@openedx/committers-frontend-app-learning this PR is ready for review |
|
I'll try to take a look by tomorrow. |
There was a problem hiding this comment.
Mostly looks good. Please just swap out courseId in favor of useContextId, and can you please update the slot README to list the new props?
| @@ -161,7 +162,13 @@ const Sequence = ({ | |||
| const defaultContent = ( | |||
| <> | |||
| <div className="sequence-container d-inline-flex flex-row w-100"> | |||
| <CourseOutlineSidebarTriggerSlot /> | |||
| <CourseOutlineSidebarTriggerSlot | |||
| courseId={courseId} | |||
There was a problem hiding this comment.
Per a big discussion on #1496 , I think courseId should not be a prop, but instead any plugins that need it can get it with the useContextId hook. See the example code for ProgressTabCourseGradeSlot which shows this.
Ideally, isStaff would be likewise always available via a useIsStaff or useUser hook, but I don't think it's been implemented/standardized yet.
I think the other props are reasonable for this.
Description
Extend CourseOutlineSidebarTriggerSlot props to amplify plugin usage.