feat: library section and subsection page #2032

navinkarkera · 2025-05-26T15:24:43Z

Description

Adds section and subsection library pages.
Refactors routing to support them and fix routing from collections page to other pages.
Refactors library context to reliably set component, unit, and other container ids even when the url changes when user goes back in history rapidly.

Useful information to include:

Which edX user roles will this change impact? Common user roles are "Learner", "Course Author"
"Developer", and "Operator".

2025-05-30.11-57-52.mp4

Supporting information

Testing instructions

Checkout feat: Support to sections/subsections dict to containers objects in index [FC-0090] openedx-platform#36806
Check the new section and subsection page as well as test opening pages via all options.

Other information

Include anything else that will help reviewers and consumers understand the change.

Does this change depend on other changes elsewhere?
Any special concerns or limitations? For example: deprecations, migrations, security, or accessibility.

openedx-webhooks · 2025-05-26T15:24:48Z

Thanks for the pull request, @navinkarkera!

This repository is currently maintained by @bradenmacdonald.

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 approval

If you haven't already, check this list to see if your contribution needs to go through the product review process.

If it does, you'll need to submit a product proposal for your contribution, and have it reviewed by the Product Working Group.
- This process (including the steps you'll need to take) is documented here.
If it doesn't, simply proceed with the next step.

🔘 Provide context

To 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:

Dependencies

This PR must be merged before / after / at the same time as ...
Blockers

This PR is waiting for OEP-1234 to be accepted.
Timeline information

This PR must be merged by XX date because ...
Partner information

This is for a course on edx.org.
Supporting documentation
Relevant Open edX discussion forum threads

🔘 Get a green build

If one or more checks are failing, continue working on your changes until this is no longer the case and your build turns green.

Details

Where 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:

The size and impact of the changes that it introduces
The need for product review
Maintenance status of the parent repository

💡 As a result it may take up to several weeks or months to complete a review and merge your PR.

navinkarkera · 2025-06-03T09:48:48Z

@rpenido Ready for another round of review.

pomegranited · 2025-06-03T13:24:56Z

src/generic/key-utils.ts

 */
 export function getBlockType(usageKey: string): string {
-  if (usageKey && usageKey.startsWith('lb:')) {
+  if (usageKey && (usageKey.startsWith('lb:') || usageKey.startsWith('lct:'))) {


This change means we don't really need getContainerTypeFromId -- since it's only really used in one place, could we move its tests over to this method and remove it entirely?

Done. aea42e8

rpenido

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

rpenido · 2025-06-03T13:42:54Z

src/generic/DraggableList/SortableItem.tsx

+  children?: React.ReactNode | null,
+  actions: React.ReactNode | null,


nit: ReactNode typing already includes null

Suggested change

children?: React.ReactNode | null,

actions: React.ReactNode | null,

children?: React.ReactNode,

actions: React.ReactNode,

rpenido · 2025-06-03T14:29:43Z

src/library-authoring/containers/HeaderActions.tsx

+      }
+    }
+    navigateTo({ [`${containerType}Id`]: containerKey });
+  }, [containerKey, infoSidebarIsOpen]);


nit

Suggested change

}, [containerKey, infoSidebarIsOpen]);

}, [containerKey, infoSidebarIsOpen, navigateTo]);

ChrisChV · 2025-06-04T03:28:56Z

@navinkarkera Do you think you can solve this issue in this task? It's the same as #2008 (comment)

ChrisChV · 2025-06-04T03:32:02Z

@navinkarkera I found the following issue:

Create two sections
Create a subsection and add it to both sections
Open one section.
Open the other section.
Edit the title of the subsection
Return to the first section. Bug: The new tittle of the subsection is not reflected, you need to refresh the page.

ChrisChV

Looks good! I left some comments and nits

src/library-authoring/data/apiHooks.ts

ChrisChV · 2025-06-04T00:06:32Z

src/library-authoring/section-subsections/LibraryContainerChildren.tsx

+      });
+    } catch (err) {
+      showToast(intl.formatMessage(containerMessages.updateContainerErrorMsg));
+      throw err;


@navinkarkera is it necessary to throw the error? Doesn't this cause errors to be displayed in the interface?

I am not sure about the reason behind it but the exception is caught here: https://github.com/open-craft/frontend-app-authoring/blob/e36b5ae0c000a34a8daf5bf4cac669e51e8e741e/src/generic/inplace-text-editor/index.tsx#L46.

Maybe @rpenido can elaborate.

This was used to rollback the optimistic update on the edit component, but I it doesn't seem to used anymore.
@navinkarkera If you have some time, could you remove this throw?
We can also remove the catch block from the component, but we need remove all throws from the handle functions.

I removed/updated this on 0658029

@rpenido Because of this broken test, I prefer to keep the catch block, for any error thrown, intentional or unintentional

src/library-authoring/section-subsections/LibraryContainerChildren.tsx

navinkarkera · 2025-06-04T05:54:49Z

@navinkarkera I found the following issue:

* Create two sections

* Create a subsection and add it to both sections

* Open one section.

* Open the other section.

* Edit the title of the subsection

* Return to the first section. Bug: The new tittle of the subsection is not reflected, you need to refresh the page.

@ChrisChV Good catch!. Fixed it by invalidating all container queries on save: a905019

navinkarkera · 2025-06-04T06:07:36Z

@navinkarkera Do you think you can solve this issue in this task? It's the same as #2008 (comment)

I'll try and create a new PR for it. Don't want to delay this one.

ChrisChV

Looks good! Great work!

openedx-webhooks added open-source-contribution PR author is not from Axim or 2U core contributor PR author is a Core Contributor (who may or may not have write access to this repo). labels May 26, 2025

openedx-webhooks added this to Contributions May 26, 2025

github-project-automation bot moved this to Needs Triage in Contributions May 26, 2025

navinkarkera mentioned this pull request May 27, 2025

feat: Basic CRUD support for sections/subsections as containers [FC-0090] openedx/openedx-platform#36762

Merged

navinkarkera force-pushed the navin/fal-4164/section-subsection-page branch from 664629a to 6b5030f Compare May 27, 2025 11:14

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions May 27, 2025

navinkarkera force-pushed the navin/fal-4164/section-subsection-page branch 2 times, most recently from acb8c17 to b5e0478 Compare May 28, 2025 15:35

navinkarkera mentioned this pull request May 30, 2025

Implement a selection state that is distinct from focus visibility. #1792

Closed

navinkarkera added 19 commits May 30, 2025 11:18

feat: library section page

5cb979d

fixup! feat: library section page

6033db1

fixup! feat: library section page

b1f318e

fixup! feat: library section page

035c7fa

fix: update children on child name update

0db3eb5

fix: navigation to newly created container

cea088c

fix: getBlockType for containers

c2114a9

style: section page

1a17ad1

fix: remove unused variable

871eff4

chore: fix message

14c8657

refactor: extract header action component

67d2c2d

refactor: extract footer actions and simplify content picker

5bebab4

refactor: imports

27dba10

feat: add subsection page and refactor id handling

b96bca3

fix: variable names

dc101ba

fix: collection page loading and remove collection option in menu

e3fad17

feat: subsection page breadcrumbs

2ee04df

refactor: component selection in container pages and add draft tag

563ae45

style: subsection breadcrumbs

5cc77a6

fix: lint issues

1bcce12

navinkarkera marked this pull request as ready for review June 3, 2025 09:48

navinkarkera requested a review from rpenido June 3, 2025 09:48

fix: show subsections tab only in add modal in section page

e5a508e

pomegranited reviewed Jun 3, 2025

View reviewed changes

rpenido approved these changes Jun 3, 2025

View reviewed changes

ChrisChV requested changes Jun 4, 2025

View reviewed changes

navinkarkera added 7 commits June 4, 2025 09:49

refactor: replace getContainerTypeFromId with getBlockType

aea42e8

fix: lint issues

4e8bc27

Merge branch 'master' into navin/fal-4164/section-subsection-page

e36b5ae

refactor: rename fn

5205375

test: empty section and subsection page

041db18

feat: invalidate all parent containers on name update

dc5f4f4

refactor: invalidate all container data on name update

a905019

navinkarkera requested a review from ChrisChV June 4, 2025 05:54

fix: lint issues

6498ff3

refactor: Delete catch on handleOnChangeText in InplaceTextEditor

0658029

ChrisChV approved these changes Jun 4, 2025

View reviewed changes

ChrisChV enabled auto-merge (squash) June 4, 2025 17:06

fix: Broken tests

1c8518e

ChrisChV merged commit dd6780f into openedx:master Jun 4, 2025
7 checks passed

github-project-automation bot moved this from Waiting on Author to Done in Contributions Jun 4, 2025

edschema mentioned this pull request Jun 4, 2025

[Bug] Scrolling behavior bug: library unit page to library browse #1992

Closed

ChrisChV mentioned this pull request Jun 14, 2025

fix: Truncated text in InplaceTextEditor [FC-0090] #2146

Merged

edschema mentioned this pull request Sep 16, 2025

[Library browse] Unintended scroll upon unit selection #2452

Closed

		children?: React.ReactNode \| null,
		actions: React.ReactNode \| null,

	}, [containerKey, infoSidebarIsOpen]);
	}, [containerKey, infoSidebarIsOpen, navigateTo]);

feat: library section and subsection page #2032

feat: library section and subsection page #2032

Uh oh!

Conversation

navinkarkera commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Supporting information

Testing instructions

Other information

Uh oh!

openedx-webhooks commented May 26, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

navinkarkera commented Jun 3, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rpenido 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!

ChrisChV commented Jun 4, 2025

Uh oh!

ChrisChV commented Jun 4, 2025

Uh oh!

ChrisChV left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

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!

Uh oh!

navinkarkera commented Jun 4, 2025

Uh oh!

navinkarkera commented Jun 4, 2025

Uh oh!

ChrisChV left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

navinkarkera commented May 26, 2025 •

edited

Loading

openedx-webhooks commented May 26, 2025 •

edited

Loading