-
Notifications
You must be signed in to change notification settings - Fork 735
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adds ability to preview non-practice resources from the sidepanel #13012
base: develop
Are you sure you want to change the base?
Conversation
Build Artifacts
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @AllanOXDi! I have left just a few comments, mainly to migrate this work to the new subPages
structure we have for the LessonResourceSelection side panel :). Please let me know if you have any question :)
@@ -155,18 +155,24 @@ export default [ | |||
path: 'channels', | |||
component: SelectFromChannels, | |||
}, | |||
{ | |||
name: PageNames.LESSON_PREVIEW_RESOURCE, | |||
path: 'preview-selected-resources', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think the path here could be just "preview" or "preview-resource" as "preview-selected-resources" is not completely acurate and can be confused with path to the shopping cart
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see that previous to this change, we had a path like this preview-resources/:nodeId
:
{
name: PageNames.LESSON_PREVIEW_RESOURCE,
path: 'preview-resources/:nodeId',
component: PreviewSelectedResources,
},
Was there any reason to change the path and the param to be a query?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think there was a particular reason to
Summary
Updated the KCard in the resource selection in lessons to navigate to preview resources.
Used the existing content renderer to display the content using the /lessonstemp/ path in the updated side panel.
Before
After
Closes #12988
References
#12988
Reviewer guidance
lessonstemp
path.Currently not implemented
Ability to add a resource to a lesson
Have not figured out what triggered the KBreadcrumbs error in the console
…