feat: create section and subsection in library - #2013
Conversation
openedx-webhooks
added
open-source-contribution
|
Thanks for the pull request, @navinkarkera! This repository is currently maintained by Once you've gone through the following steps feel free to tag them in a comment and let them know that your changes are ready for engineering review. 🔘 Get product approvalIf you haven't already, check this list to see if your contribution needs to go through the product review process.
🔘 Provide contextTo help your reviewers and other members of the community understand the purpose and larger context of your changes, feel free to add as much of the following information to the PR description as you can:
🔘 Get a green buildIf one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green. DetailsWhere can I find more information?If you'd like to get more details on all aspects of the review process for open source pull requests (OSPRs), check out the following resources: When can I expect my changes to be merged?Our goal is to get community contributions seen and reviewed as efficiently as possible. However, the amount of time that it takes to review and merge a PR can vary significantly based on factors such as:
💡 As a result it may take up to several weeks or months to complete a review and merge your PR. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #2013 +/- ##
========================================
Coverage 93.98% 93.98%
========================================
Files 1155 1155
Lines 24146 24176 +30
Branches 5116 5222 +106
========================================
+ Hits 22693 22723 +30
+ Misses 1385 1376 -9
- Partials 68 77 +9 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
c5cf00a to
54ada1b
Compare
rpenido
left a comment
rpenido
left a comment
There was a problem hiding this comment.
LGTM 👍
Thank you for your work, @navinkarkera!
- I tested this using the instructions from the PR
- I read through the code
- I checked for accessibility issues
- Includes documentation
I just added some nits.
| createUnitModalNamePlaceholder: { | ||
| id: 'course-authoring.library-authoring.modals.create-unit.form.name.placeholder', | ||
| defaultMessage: 'Give a descriptive title', | ||
| description: 'Placeholder of the Name field of the Create Unit modal form', | ||
| }, | ||
| createSubsectionModalNamePlaceholder: { | ||
| id: 'course-authoring.library-authoring.modals.create-subsection.form.name.placeholder', | ||
| defaultMessage: 'Give a descriptive title', | ||
| description: 'Placeholder of the Name field of the Create Subsection modal form', | ||
| }, | ||
| createSectionModalNamePlaceholder: { | ||
| id: 'course-authoring.library-authoring.modals.create-section.form.name.placeholder', | ||
| defaultMessage: 'Give a descriptive title', | ||
| description: 'Placeholder of the Name field of the Create Section modal form', | ||
| }, |
There was a problem hiding this comment.
What do you think about removing the common strings here?
| createUnitModalNamePlaceholder: { | |
| id: 'course-authoring.library-authoring.modals.create-unit.form.name.placeholder', | |
| defaultMessage: 'Give a descriptive title', | |
| description: 'Placeholder of the Name field of the Create Unit modal form', | |
| }, | |
| createSubsectionModalNamePlaceholder: { | |
| id: 'course-authoring.library-authoring.modals.create-subsection.form.name.placeholder', | |
| defaultMessage: 'Give a descriptive title', | |
| description: 'Placeholder of the Name field of the Create Subsection modal form', | |
| }, | |
| createSectionModalNamePlaceholder: { | |
| id: 'course-authoring.library-authoring.modals.create-section.form.name.placeholder', | |
| defaultMessage: 'Give a descriptive title', | |
| description: 'Placeholder of the Name field of the Create Section modal form', | |
| }, | |
| createContainerModalNamePlaceholder: { | |
| id: 'course-authoring.library-authoring.modals.create-container.form.name.placeholder', | |
| defaultMessage: 'Give a descriptive title', | |
| description: 'Placeholder of the Name field of the Create Container modal form', | |
| }, |
There was a problem hiding this comment.
@rpenido Kept them separate for translators to provide different translations if required for section, subsection and units.
54ada1b to
0f3a45d
Compare
| // "Create New Section" modal | ||
| isCreateSectionModalOpen: boolean; | ||
| openCreateSectionModal: () => void; | ||
| closeCreateSectionModal: () => void; | ||
| // "Create New Subsection" modal | ||
| isCreateSubsectionModalOpen: boolean; | ||
| openCreateSubsectionModal: () => void; | ||
| closeCreateSubsectionModal: () => void; | ||
| // "Create New Unit" modal | ||
| isCreateUnitModalOpen: boolean; | ||
| openCreateUnitModal: () => void; |
There was a problem hiding this comment.
@navinkarkera what do you think about merging these values and functions? All are containers and open the same modal
There was a problem hiding this comment.
Hmm, was not so sure about it. Is this better: f6e3a83?
There was a problem hiding this comment.
Looks good for me. What are your concerns?
There was a problem hiding this comment.
@ChrisChV No, it looks good now. Initially, I did not want to replace toggle with state, but it looks fine.
75cc038 to
f6e3a83
Compare
666a80f to
0683ad2
Compare
0683ad2 to
a147170
Compare
a147170 to
dd49ded
Compare
5 participants
Description
Adds create section and subsection buttons in sidebar.
Useful information to include:
Supporting information
Private-ref: https://tasks.opencraft.com/browse/FAL-4170Testing instructions
Newbutton in library authoring page.Other information
Include anything else that will help reviewers and consumers understand the change.