feat: Add tags to a taxonomy by jesperhodge · Pull Request #2872 · openedx/frontend-app-authoring

jesperhodge · 2026-02-12T15:02:11Z

Description

This is a PR in Draft status to address Modular-learning #132: Adding functionality to create tags from a taxonomy list.

Go to /authoring/taxonomy, open a taxonomy, and you should be able to create new tags.

Architecture

The previously used Paragon DataTable is not designed to allow in-line edit functionality or work well with trees / deeply nested table entries. So I used tanstack/react-table directly to build a new tree-table that is editable inline.

AI Usage

To speed things up, I have heavily worked with Github Copilot. I have reviewed all the code carefully, but I want to point that out for awareness when it comes to review. I created pretty exhaustive tests to ensure that the code works as expected.

What is implemented so far

Basic table functionality
Expanding rows
Creating tags
Creating subtags
After successfully saving new tags, there is an internal "Preview" mode that displays changes to the table in-place without refreshing and resorting data in the backend

What still needs to be implemented

Fix pagination: right now a row's subtags may be on different pages, which creates problems
Everything related to visual design
Column layout
Missing dropdown / menus
Column sorting
Accessibility
Clean up tests

openedx-webhooks · 2026-02-12T15:02:18Z

Thanks for the pull request, @jesperhodge!

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.

🔘 Update the status of your PR

Your PR is currently marked as a draft. After completing the steps above, update its status by clicking "Ready for Review", or removing "WIP" from the title, as appropriate.

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.

jesperhodge · 2026-03-03T22:34:04Z

Dear reviewers,
there is a lot to do in this PR, since I had to build a new table for this. The core functionality is done, so I would love to already get an early review and feedback, even though this is unfinished. In the interest of getting this out the door soon.

bradenmacdonald · 2026-03-03T23:05:15Z

Thanks, I'm so happy to see this coming together!

Fix pagination: right now a row's subtags may be on different pages, which creates problems

Any thoughts on how to handle this?

Would it be useful to use infinite scroll instead of pagination, and load sub-tags on demand when their parent tag is expanded?

After successfully saving new tags, there is an internal "Preview" mode that displays changes to the table in-place without refreshing and resorting data in the backend

This doesn't seem to be working. When I add a new top-level tag, it always appears at the top of the list, but when I later refresh, it moves to alphabetical order. I think it should immediately put the tag into the correct position and then "flash" it to highlight where it is in the list.

Bugs:

When I add a new top-level tag then add a sub-tag, the top-level tag incorrectly still says "(0)" subtags until I refresh the page.

Here's a bunch of UX feedback. I know you're probably aware of many of these already, and they don't have to be fixed within this PR necessarily, but it's easier for me to just list them all.

It seems odd that "Expand Row" is only implemented for top-level tags. Can we expand/collapse the sub-tags in the future too?
"Expand Row" button would be better as a chevron up/down icon, like on the standard Paragon Collapsible.
"New Tag" button should be alongside the "Expand All" button
Clicking the "three dots" button on a tag/subtag should open a menu like this instead of revealing a button. Revealing the "Add subtag" button causes the whole table layout to shift.
The "three dots" buttons should all be aligned instead of scattered based on tag depth.
Whichever row you hover over could be highlighted.
If we only allow tags to be three levels deep, there is no need to display "(0)" after grandchild tags, as they can never have children.

bradenmacdonald

Don't have time for a full review now, but here's some initial thoughts.

src/taxonomy/tag-list/tagColumns.tsx

bradenmacdonald · 2026-03-03T23:12:41Z

src/taxonomy/tag-list/TagListTable.tsx

+  // The table has a VIEW, DRAFT, and a PREVIEW mode. It starts in VIEW mode.
+  // It switches to DRAFT mode when a user edits or creates a tag.
+  // It switches to PREVIEW mode after saving changes, and only switches to VIEW when
+  // the user refreshes the page, orders a column, or navigates to a different page.
+  // During DRAFT and PREVIEW mode the table makes POST requests and receives
+  // success or failure responses.
+  // However, the table does not refresh to show the updated data from the backend.
+  // This allows us to show the newly created or updated tag in the same place without reordering.


I wonder if we can take a simpler approach. What do you think about having just one "mode", and using optimistic updates to inject any newly-created tags into the correct spot? That way, if/when react-query refetches data from the backend, nothing gets disrupted, and we can keep everything in sync.

(plus a toggle to track whether the user is currently creating a new tag or not)

The modes are mostly necessary to prevent reloading data and alphabetical reloading after the user has successfully saved a new tag, so that new tags and subtags are shown at the top. See my larger comment about this criteria. In terms of optimistic updates, which are there to inject updates even before they have been saved, that would prevent tags to show up in the spot we want, and it doesn't align with our AC, which is to show error messages when the tag does not successfully save and not display the tag optimistically hoping for successful save.

jesperhodge · 2026-03-04T16:02:39Z

@bradenmacdonald thanks for the very helpful feedback!

Pagination:
I was thinking of changing that behavior in the backend paginator, as I'd like to paginate by 0th-level tags. While nested tags are included because of the full_depth_threshold parameter, I would expect those not to count towards page size. Do you have any concerns about that? I admit it's a bit weird because they are all in one flat array.

Otherwise, as an alternative, as you said, is to load all child / grandchildren tags when we expand the table. Since we have an "Expand All" functionality, and multiple levels, and I don't want to spawn thousands of requests, I think that would need to be one bulk request. So we have initial (paginated) request for top-level tags, and then one second request filtering for only the top-level tags from the current page and loading them unpaginated together with their children.

Tags ordering / Preview:

When I add a new top-level tag, it always appears at the top of the list, but when I later refresh, it moves to alphabetical order. I think it should immediately put the tag into the correct position and then "flash" it to highlight where it is in the list.

I implemented that as we decided for the ACs. The current way it works is indeed that it always appears at the top of the list, but on refresh moves to alphabetical order. Some of the reasons we landed on these ACs were:

In a typical user workflow, if they are adding a top-level tag, they will probably want to add sub-tags at that time. To make that process easy, the tag is displayed at the top of the list, allowing them to quickly add sub-tags.
When sorted alphabetically, if the new tag is not displayed on the first page of the pagination, they will have to hunt for the new tag and will not see where it goes, even if it flashes in some way, because they are not currently on that page.

I brought your recommendation about positioning and flashing to the team, and we are going to raise this concern to the product owner, Jenna, to get her input.

Bugs:

good find about sub-tags number - going to fix that.

UX:

Thanks for listing these UX things. UX for the table is not implemented at all yet, and I plan to fully finish that right here before marking this PR as ready.

Let me know if you have any questions! Your feedback is very valuable to us.

tbain · 2026-03-04T17:39:00Z

src/taxonomy/data/apiHooks.ts

    queryKey: taxonomyQueryKeys.taxonomyTagListPage(taxonomyId, pageIndex, pageSize),
    queryFn: async () => {
-      const { data } = await getAuthenticatedHttpClient().get(apiUrls.tagList(taxonomyId, pageIndex, pageSize));
+      const { data } = await getAuthenticatedHttpClient().get(apiUrls.tagList(taxonomyId, pageIndex, pageSize, 1000));


What is the "1000" here? Do we have a place to make this a constant somewhere?

tbain · 2026-03-04T17:43:24Z

src/taxonomy/tag-list/hooks.ts

+    return 'Name is required';
+  }
+  if (!TAG_NAME_PATTERN.test(trimmed)) {
+    return 'Invalid character in tag name';


Should this return a guide to fix the issue? e.g. "Invalid character in tag name, allowed characters are uppercase letters, lowercase letters, and numbers" or something to that effect? If we have a guide somewhere else on that page aside from this that's fine.

tbain · 2026-03-04T18:22:20Z

src/taxonomy/tag-list/tagTree.ts

+  private validateNoDuplicateValues(items: TagData[]) {
+    const seenValues = new Set<string>();
+    for (const item of items) {
+      if (seenValues.has(item.value)) {


Does we want this to be case insensitive? So "tuba" would be a duplicate of "Tuba" and "TuBa"?

tbain · 2026-03-04T18:24:03Z

src/taxonomy/taxonomy-detail/TaxonomyDetailPage.jsx

          >
            <Layout.Element>
-              <TagListTable taxonomyId={taxonomyId} />
+              <TagListTable taxonomyId={taxonomyId} maxDepth={3} />


Is there a place to move this to a configuration so that if we want to change the depth it doesn't require a code change?

bradenmacdonald · 2026-03-04T18:31:04Z

When sorted alphabetically, if the new tag is not displayed on the first page of the pagination, they will have to hunt for the new tag and will not see where it goes, even if it flashes in some way, because they are not currently on that page.

If we implement "flashing", we could also implement "change to the correct page".

I was thinking of changing that behavior in the backend paginator, as I'd like to paginate by 0th-level tags. While nested tags are included because of the full_depth_threshold parameter, I would expect those not to count towards page size. Do you have any concerns about that? I admit it's a bit weird because they are all in one flat array.

It's fine with me if you add the option to paginate by 0th-level tags, but I think it may need to be optional, in order to preserve the API compatibility.

The current API allows you to quickly load the entire taxonomy into memory by requesting the full depth and as many pages as you need until it's complete, and I think that's another option to consider here unless we think there will be taxonomies too large to performantly display in a react-table.

tbain · 2026-03-04T18:54:48Z

src/taxonomy/tag-list/hooks.ts

+      const parentTag = parentTagValue ? nextTree.getTagAsDeepCopy(parentTagValue) : null;
+
+      nextTree.addNode({
+        id: Date.now(),


We discussed this via a side channel, but do we have any concerns with this being just a timestamp value rather than something like a GUID or UUID (Or a numerical only equivalent) that is better guaranteed to be unique? I know the timestamp is 99.99...% unique in real world cases, but just wanted to make sure we didn't want to use something more securely unique.

I can't imagine a scenario where there would be a conflict.

feat: add a new tag from frontend

70bece4

openedx-webhooks added the open-source-contribution PR author is not from Axim or 2U label Feb 12, 2026

openedx-webhooks added this to Contributions Feb 12, 2026

github-project-automation bot moved this to Needs Triage in Contributions Feb 12, 2026

jesperhodge changed the title ~~feat: Add tags to a taxonomy #132~~ feat: Add tags to a taxonomy Feb 12, 2026

feat: Add table control bar with expand button

e6d6cb7

mphilbrick211 moved this from Needs Triage to Waiting on Author in Contributions Feb 18, 2026

jesperhodge added 22 commits February 19, 2026 13:52

feat: create tags

e70becf

feat: use react-table and get full depth of tags

3d9188b

feat: support nested subrows in tag list table

39b8438

feat: can create new tags with a subtag as parent

ba08de0

feat: show add row conditionally on table depth

f11e813

test: make existing tag list table tests work again

54b1175

test: create tags

de4e8c5

test: generate tests from acceptance criteria

3ea6bed

test: add tests for nested sub-tags and taxonomy editability

0f4dfda

feat: keep table working state with new row at top

15e9de1

feat: add tag tree data structure

ec926db

feat: create tag tree

a617cb8

fix: creating top tags

f5f1ffc

feat: add card style

6021b82

feat: add plus icon

38df0b7

add button styling

9d150e7

test: fix tests that are implemented

f586ce3

feat: add reducer for table modes

e6ba153

feat: enable preview mode

6a89564

fix: mode transitions

a737e6c

feat: add row options menu

7b1223d

test: skip anything thats not implemented yet

5d34a3a

jesperhodge added 11 commits February 27, 2026 19:38

test: fix test

ec69a28

refactor: change table mode name to preview

19690ee

refactor: extract subcomponents

fa240e7

fix: tests

91c7d80

refactor: extract table display component

a689803

refactor: make table components reusable

079a681

refactor: simplify and extract components

b15f3e6

refactor: extract reusable tree table components

51f39e9

refactor: convert to typescript

3c4bcac

refactor: make tree table more readable

57fe113

Merge remote-tracking branch 'upstream/master' into jhodge/create-tags

3f61fe4

bradenmacdonald reviewed Mar 3, 2026

View reviewed changes

fix: delete duplicate file

f9703ea

tbain reviewed Mar 4, 2026

View reviewed changes

Conversation

jesperhodge commented Feb 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Architecture

AI Usage

What is implemented so far

What still needs to be implemented

Uh oh!

openedx-webhooks commented Feb 12, 2026

Uh oh!

jesperhodge commented Mar 3, 2026

Uh oh!

bradenmacdonald commented Mar 3, 2026

Uh oh!

bradenmacdonald left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jesperhodge commented Mar 4, 2026

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!

bradenmacdonald commented Mar 4, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

jesperhodge commented Feb 12, 2026 •

edited

Loading