feat: complete subsection & unit features by navinkarkera · Pull Request #797 · openedx/frontend-app-authoring

navinkarkera · 2024-01-16T13:31:43Z

Description:

This PR combines following features and PRs:

feat: Add drag and drop support to subsections #769 Private-ref BB-8278
feat: use actions and other flags to control item actions in outline #781 Private-ref BB-8431
feat: add colored left border to items in course outline #788 Private-ref BB-8421
feat: Add drag-n-drop support to course unit, refactor tests. open-craft/frontend-app-authoring#27 Private-ref BB-8423
feat: add subsection configuration modal open-craft/frontend-app-authoring#22 Private-ref BB-8277
Feat: add Unit Configuration Modal open-craft/frontend-app-authoring#26 Private-ref BB-8430

feat: move up & down option Private-ref BB-8422

Description:

Adds move up and down option to sections, subsections and units menu.

Screenshots:

Test instructions:

Get devstack up and running using make {lms,cms}-up.
Start npm devserver in this repo using npm start.
Goto http://localhost:2001/course/course-v1:edX+DemoX+Demo_Course/ and click on three dots of any section or subsection.
It should move sections/subsections as expected.
Options should be visible based on the position, like first section should not have an option to move up etc.

Notes: Please use rebase option to merge (Do not squash, as the commits are already squashed based on the features and authors).

openedx-webhooks · 2024-01-16T13:31:48Z

Thanks for the pull request, @navinkarkera! Please note that it may take us up to several weeks or months to complete a review and merge your PR.

Feel free to add as much of the following information to the ticket as you can:

supporting documentation
Open edX discussion forum threads
timeline information ("this must be merged by XX date", and why that is)
partner information ("this is a course on edx.org")
any other information that can help Product understand the context for the PR

All technical communication about the code itself will be done via the GitHub pull request interface. As a reminder, our process documentation is here.

Please let us know once your PR is ready for our review and all tests are green.

codecov · 2024-01-16T13:40:13Z

Codecov Report

Attention: 35 lines in your changes are missing coverage. Please review.

Comparison is base (3842b04) 90.30% compared to head (2f6af06) 90.46%.

Files	Patch %	Lines
...c/course-outline/configure-modal/VisibilityTab.jsx	50.00%	13 Missing ⚠️
src/course-outline/configure-modal/AdvancedTab.jsx	86.11%	5 Missing ⚠️
src/course-outline/utils.jsx	78.26%	5 Missing ⚠️
.../course-outline/configure-modal/ConfigureModal.jsx	95.18%	4 Missing ⚠️
src/course-outline/CourseOutline.jsx	95.00%	3 Missing ⚠️
src/course-outline/configure-modal/UnitTab.jsx	86.36%	3 Missing ⚠️
src/course-outline/hooks.jsx	91.30%	2 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master     #797      +/-   ##
==========================================
+ Coverage   90.30%   90.46%   +0.15%     
==========================================
  Files         506      509       +3     
  Lines        8476     8859     +383     
  Branches     1782     1903     +121     
==========================================
+ Hits         7654     8014     +360     
- Misses        794      817      +23     
  Partials       28       28

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

KristinAoki

What waffle flags control the deletable, draggable, duplicable, and childAddable?

KristinAoki · 2024-01-16T20:14:04Z

+                {actions.allowMoveUp && (
+                  <Dropdown.Item
+                    data-testid={`${namePrefix}-card-header__menu-move-up-button`}
+                    onClick={onClickMoveUp}
+                  >
+                    {intl.formatMessage(messages.menuMoveUp)}
+                  </Dropdown.Item>
+                )}
+                {actions.allowMoveDown && (


For the move up and down would it be better to disable the option when they are at the top and bottom rather than removing it from the list of options. I think it is best to keep the menus consitent when moving them around.

Will update it.

Update: Done

KristinAoki · 2024-01-16T20:40:46Z

-  SortableItem,
  ErrorAlert,
 } from '@edx/frontend-lib-content-components';
+import { arrayMove } from '@dnd-kit/sortable';


Will this cause any dependency conflict issues with frontend-lib-content-components if the package versions do not match? Also what does arrayMove do?

I think npm should take care of it as @dnd-kit/sortable is already part of frontend-lib-content-components.

As for arrayMove function, it handles moving an element inside an array to a new index. It is used in drag-n-drop implementation in frontend-lib-content-components.

What is the reason for adding it here if the function is already available in frontend-lib-content-components?

@KristinAoki Assuming that you mean adding the dependency in this repo, the linter complains if we don't explicitly add the dependency here and it makes sense as there is no guarantee that frontend-lib-content-components won't remove it in future.

navinkarkera · 2024-01-17T12:18:34Z

What waffle flags control the deletable, draggable, duplicable, and childAddable?

@KristinAoki They are not controlled via waffle flags but this part of the API adds it.

navinkarkera · 2024-01-17T12:30:42Z

@KristinAoki Although, I have updated the PR, we haven't done an internal review yet so I would recommend you to wait for now. I'll ping you in a combined PR when it is ready. Apologies for the confusion.

cc: @mphilbrick211

mavidser

👍 LGTM. One minor note

I tested the move up and down behavior in sections and subsections
I read through the code
I checked for accessibility issues

navinkarkera · 2024-01-18T14:21:50Z

@mavidser Also added move menu actions to units after including commits from unit drag-n-drop implementation.

mavidser · 2024-01-19T09:47:30Z

@navinkarkera LGTM 👍

openedx-webhooks · 2024-01-19T10:13:29Z

I've created issue TNL-11377 in the private 2U Jira.

navinkarkera · 2024-01-19T10:13:59Z

@KristinAoki This is ready for you. Combined all PRs implemented in last 2 weeks here.

KristinAoki

As I was going through the menu options for sections, subsections, and units, I realized that the messages need to be changed in the publish modals. The headers are good, but the body of the modal are incorrect. The following changes need to be made:

Publish all unpublished changes for this chapter? -> Publish all unpublished changes for this section?
Publish all unpublished changes for this sequential? -> Publish all unpublished changes for this subsection?
Publish all unpublished changes for this vertical? -> Publish all unpublished changes for this unit?

KristinAoki · 2024-01-22T16:08:31Z

+        <div className="hide-header mt-3" data-testid="advanced-tab-hours-picker-wrapper">
+          <DatepickerControl
+            type={DATEPICKER_TYPES.time}
+            value={timeLimit}


Based on the helper text, the time limit should be able to enter a time greater than 23:59 such as 72:00

Nice catch! Updated to use normal form autosuggest as datetime picker does not allow time greater than 24:00.

I would just update it use Form control because the autosuggest is not necessary and crowds the modal. If we are worried about users not using the correct format for the time, we could add "HH:MM" as the placeholder for the field or set a pattern for the field.

@KristinAoki That makes the code simpler. Added a form control with pattern validation as well as placeholder.

KristinAoki · 2024-01-22T16:26:18Z

  subsection,
  children,
+  index,
+  canMoveItem,


Should this be false when there are no other siblings, such as when there is only on subsection in a section? There is no where to move the element so it shouldn't have the drag handle or the move options in the menu.

Should this be false when there are no other siblings, such as when there is only on subsection in a section?

Yes, both move up & down options are disabled.

There is no where to move the element so it shouldn't have the drag handle

Makes sense, updated 👍

KristinAoki · 2024-01-22T16:30:19Z

+              setDueDate={setDueDateState}
+            />
          </Tab>
          <Tab eventKey="visibility" title={intl.formatMessage(messages.visibilityTabTitle)}>


When viewing this tab I would expect the modal to scroll instead of the entire page. The expected scroll behavior can be seen in the highlights modal

navinkarkera

@KristinAoki Addressed all your comments 👍 .

navinkarkera · 2024-01-23T09:56:11Z

+        <div className="hide-header mt-3" data-testid="advanced-tab-hours-picker-wrapper">
+          <DatepickerControl
+            type={DATEPICKER_TYPES.time}
+            value={timeLimit}


Nice catch! Updated to use normal form autosuggest as datetime picker does not allow time greater than 24:00.

navinkarkera · 2024-01-23T10:27:41Z

  subsection,
  children,
+  index,
+  canMoveItem,


Should this be false when there are no other siblings, such as when there is only on subsection in a section?

Yes, both move up & down options are disabled.

There is no where to move the element so it shouldn't have the drag handle

Makes sense, updated 👍

feat: Update tests, fix bugs in drag and drop elements chore: address review feedback

Uses action flags from API to control display of delete, duplicate, child new button and dragging. Use isHeaderVisible flag to control display of subsection headers. All these changes prepare outline for entrance exam section display. feat: use actions flags for subsections test: actions

refactor: move common styles to conditional sortable element

chore: address review feedback

test: add tests for move options refactor: disable move option instead of hiding fix: incorrect variable name in tests feat: move up & down menu action for units test: add tests for unit move options

test: add render and API tests fix: fix non saving options and add review style changes fix: remove additional tab in the section configuration fix: remove isSubsection state, fix css issues and fix tests fix: add review changes, fix advanced tab hour selection and update tests test: fix failing test in courseOutline.test.jsx fix: remove unused state, add TODO comment, fix stack rendering and NaN values feat: show previous state in autosuggest if an invalid option is provided and fix warnings test: fix failing test

fixes issues related to form autosuggest fix: hide header only for advanced tab time picker

openedx-webhooks · 2024-01-24T14:50:20Z

@navinkarkera 🎉 Your pull request was merged! Please take a moment to answer a two question survey so we can improve your experience in the future.

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Jan 16, 2024

navinkarkera marked this pull request as ready for review January 16, 2024 13:51

mphilbrick211 requested a review from a team January 16, 2024 15:17

KristinAoki reviewed Jan 16, 2024

View reviewed changes

navinkarkera force-pushed the navin/course-outline/move-action branch from 942b8ad to 05f8ee4 Compare January 17, 2024 12:28

mavidser approved these changes Jan 17, 2024

View reviewed changes

Comment thread src/course-outline/CourseOutline.test.jsx Resolved

navinkarkera force-pushed the navin/course-outline/move-action branch 2 times, most recently from ba9c17b to a2ba99c Compare January 18, 2024 14:11

navinkarkera changed the title ~~feat: move up and down menu action for sections and subsections~~ feat: move up and down menu action for sections, subsections and units Jan 18, 2024

navinkarkera force-pushed the navin/course-outline/move-action branch from a2ba99c to ef1096e Compare January 19, 2024 04:59

navinkarkera changed the title ~~feat: move up and down menu action for sections, subsections and units~~ feat: complete subsection & unit features Jan 19, 2024

navinkarkera added the jira:2u We want an issue in the 2U Jira instance label Jan 19, 2024

KristinAoki suggested changes Jan 22, 2024

View reviewed changes

navinkarkera force-pushed the navin/course-outline/move-action branch from 2b74f41 to 996ee5a Compare January 23, 2024 10:53

navinkarkera commented Jan 23, 2024

View reviewed changes

navinkarkera requested a review from KristinAoki January 23, 2024 10:54

navinkarkera force-pushed the navin/course-outline/move-action branch from 996ee5a to 6f39842 Compare January 24, 2024 11:26

navinkarkera force-pushed the navin/course-outline/move-action branch 2 times, most recently from 8b5b9a8 to e92a6dc Compare January 24, 2024 11:41

Sid Verma and others added 11 commits January 24, 2024 17:13

feat: Add drag and drop support to subsections

fc3eea0

feat: Update tests, fix bugs in drag and drop elements chore: address review feedback

refactor: course outline badge status logic

a74ff9c

feat: add colored left border to items in course outline

5c17783

refactor: move common styles to conditional sortable element

feat: Add drag-n-drop support to course unit, refactor tests.

09ed9fb

chore: address review feedback

feat: move up & down menu action for sections, subsections & units

45f2f0f

test: add tests for move options refactor: disable move option instead of hiding fix: incorrect variable name in tests feat: move up & down menu action for units test: add tests for unit move options

refactor: use time picker in advanced tab

6165f3e

fixes issues related to form autosuggest fix: hide header only for advanced tab time picker

feat: add unit configuration modal

534f6cb

test: improve coverage for section, subsection & unit configuration

7d53fcd

refactor: replace time picker by text box in advanced tab

2f6af06

navinkarkera force-pushed the navin/course-outline/move-action branch from e92a6dc to 2f6af06 Compare January 24, 2024 11:43

KristinAoki approved these changes Jan 24, 2024

View reviewed changes

KristinAoki merged commit 09eef60 into openedx:master Jan 24, 2024

xitij2000 deleted the navin/course-outline/move-action branch February 2, 2024 04:54

javoconsultant mentioned this pull request Oct 16, 2026

feat: scope competency associations to the listed courses (#683) AShatsila/frontend-app-authoring#6

Open

7 tasks done

mgwozdz-unicon mentioned this pull request Oct 23, 2026

feat: competency criteria data layer and associations context(#672-01) #3275

Open

6 of 7 tasks

javoconsultant mentioned this pull request Oct 29, 2026

feat: Implement blocked courses feature for competency associations javoconsultant/frontend-app-authoring#4

Draft

7 tasks done

Conversation

navinkarkera commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

openedx-webhooks commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov Bot commented Jan 16, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

KristinAoki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navinkarkera Jan 17, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navinkarkera Jan 18, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navinkarkera commented Jan 17, 2024

Uh oh!

navinkarkera commented Jan 17, 2024

Uh oh!

mavidser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

navinkarkera commented Jan 18, 2024

Uh oh!

mavidser commented Jan 19, 2024

Uh oh!

openedx-webhooks commented Jan 19, 2024

Uh oh!

navinkarkera commented Jan 19, 2024

Uh oh!

KristinAoki left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

navinkarkera left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

openedx-webhooks commented Jan 24, 2024

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

navinkarkera commented Jan 16, 2024 •

edited

Loading

openedx-webhooks commented Jan 16, 2024 •

edited

Loading

codecov Bot commented Jan 16, 2024 •

edited

Loading

navinkarkera Jan 17, 2024 •

edited

Loading

navinkarkera Jan 18, 2024 •

edited

Loading