Skip to content

feat: Add component to Unit [FC-0083] - #82

Closed
ChrisChV wants to merge 27 commits into
navin/fal-4049/library-unit-pagefrom
chris/FAL-4058-add-component-to-unit
Closed

feat: Add component to Unit [FC-0083]#82
ChrisChV wants to merge 27 commits into
navin/fal-4049/library-unit-pagefrom
chris/FAL-4058-add-component-to-unit

Conversation

@ChrisChV

@ChrisChV ChrisChV commented Apr 9, 2025

Copy link
Copy Markdown
Member

Description

  • Creation workflow in unit page.
  • Which edX user roles will this change impact? "Course Author".

image

Supporting information

Testing instructions

  • Go to the library home of a library.
  • Create a Unit.
  • Open the Unit page of the new unit.
  • Click on Add content button.
  • Create different blocks.
  • Verify that the blocks are created and appear on the unit page.

Other information

N/A

navinkarkera and others added 27 commits April 8, 2025 09:17
Adds block tiles to the Unit card to indicate type and quantity of children in the container.
to avoid confusion with library containers. The AddContent is used to add content to libraries, collections and containers.
@ChrisChV ChrisChV changed the title Chris/fal 4058 add component to unit feat: Add component to Unit [FC-0083] Apr 9, 2025
@ChrisChV
ChrisChV marked this pull request as draft April 9, 2025 01:23

@pomegranited pomegranited left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Working great @ChrisChV ! Just one missing button and a code organization nit.

  • I tested this using the PR test instructions.
  • I read through the code
  • I checked for accessibility issues by using my keyboard to navigate
  • Includes documentation of new functions and tests.
  • User-facing strings are extracted for translation

onClick={openAddContentSidebar}
>
{intl.formatMessage(messages.newContentButton)}
</Button>

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the ticket, it looks like we need to show an additional "Add New Component" button under the list of blocks in a unit (in addition to the "Add Content" button at the top).

@@ -0,0 +1,21 @@
/**

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: Could you move this code to src/generic/key-utils.ts (and add a test)?

@navinkarkera
navinkarkera force-pushed the navin/fal-4049/library-unit-page branch 4 times, most recently from c14509d to ea853aa Compare April 11, 2025 06:54

@rpenido rpenido left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍
Thank you for your work, @ChrisChV !

  • I tested this using the instructions from the PR
  • I read through the code
  • I checked for accessibility issues
  • Includes documentation

Add some non-blocking nits!

Comment thread src/library-authoring/data/apiHooks.ts
return undefined;
},
onSettled: () => {
queryClient.invalidateQueries({ queryKey: containerQueryKeys.children(containerId) });

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since we show the component count on the ContainerCard, we need to invalidate the whole library here.

Also, it is unrelated to this task, but adding a component does not update the search index. cc @pomegranited
image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants