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

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

Description

This addresses 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.

Acceptance Criteria

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 not in scope

Fix pagination: right now a row's subtags may be on different pages, which creates problems. So pagination has been disabled for now.
Column sorting
Highlighting newly created rows

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.

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.

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.

Can I resolve this conversation?

I still think the modes are adding a lot of unnecessary complexity. Couldn't we achieve the same thing without these modes, and a much simpler isCurrentlyEditingTag and lastCreatedTag state that correspond to the "draft mode" and "preview mode" respectively?

If you track those two states, then it doesn't matter if/when React query reloads data, and you don't have to have special case behavior or state transitions, or anything else. You just make sure that if there is currently a lastCreatedTag (you'd track both the tag and its parent ID in the state), then it gets "hidden" from its normal place and rendered directly below its parent. I think that would achieve all the AC you have.

You could also do it with optimistic updates too; just because you're using optimistic updates doesn't mean you can't insert the new tag into the desired place (first on the list) or handle errors when they eventually come through. But in that case, it would be more work to avoid the eventual shift into alphabetical order whenever React query does refresh the list, which is trivially avoided by the state approach I suggested above.

I know you're short on time here so I won't block the PR on this but I do think it should be at least marked as a TODO to explore, as a way of simplifying this.

I love these ideas, and put them into a TODO. I would also like to simplify the table as much as possible.
I'd like to explore these ideas more when I find some time in the future.

One question: are you sure these two ideas make the table less complex? My take at least with states like isCurrentlyEditing is that the more states like this we have, the more complex the situation becomes. My goal with the table modes was to abstract from these details and just make the table behavior easy to predict and very robust, that's why the state machine and the safeguards. The overall TagListTable file is now very short and has little state and few useEffects, making the lifecycle quite simple.

I agree that the modes make things less simple, and if we can get rid of them and simplify I would be glad. But wouldn't either of the suggested solutions introduce more complicated table states?

Hmm yes I mean always fetching the data and then hiding or displaying the tags where we want them will be a bit simpler, but introduce some divergence between the data state and what the table displays.
I'm actually already using an approach like this, if you think about it. I separated data that is fetched to data as it is displayed in the frontend. Basically "hiding" a tag and "displaying" it in a different place is just done by editing the tagTree, which represents the displayable frontend state, and is very predictable. You just tell it where to add or remove a node and it's done.
In generally there's no need to prevent the data fetching - we could also always fetch the data but just not update the displayed data in the table. However I was thinking that would just make everything more unclear.

By the way I'm not meaning to push back on your ideas, I'm just brainstorming to figure out what the best solution would be. I'm principally not opposed to either of your suggestions.

I guess I was hoping it would be much less complicated, and the only differences in behavior would be limited to the NestedRows component, but I'll defer to your opinion on it.

The main thing about lastCreatedTag is it doesn't have any global effect; it just affects the parent of the lastCreatedTag (which you'd store in the state), so that within the NestedRows component, you'd have a check that re-orders a local copy of its childRowsData if parentRowValue == lastCreatedTag.parentValue.

So I'm thinking it would just be a few lines of code in <NestedRows>, but maybe I am missing some other behavior that we need.

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?

It's pretty arbitrary, it's the recommendation in the API docs if you want to request all depths (since there's no "infinite"). I'll just add a comment that this fetches full depth if that's fine.

@jesperhodge I agree that we may want to make this a constant in each of the places it's used.

src/taxonomy/tag-list/hooks.ts

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"?

No, this should be case sensitive, because:

I don't know if the backend allows duplicates if they have different cases.

This function is for error handling when building the tag tree, to tell us what happened, not for input validation.

The input validation lives in tag-list/hooks.ts and does not check for duplication at all, because we let the backend handle this when we hit "Save".

Can we resolve this conversation?

The backend does not allow duplicates that have different cases.

This function is for error handling when building the tag tree, to tell us what happened, not for input validation.

Do you mean it's like a debug assertion? Or under what circumstances would this be necessary? The database enforces that tags are case-insensitively unique so it's really not likely to happen.

This is just robust error handling. If the backend data is not valid or a developer writes code that malforms data in some edge cases, there is a clear error message that can be picked up by bug tracking systems and clearly indicates where the problem is. That way nobody needs to debug the tagTree when the error is just in the data being passed into it.

Fine, please just explain that in the code.

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?

Yes, I'll move that to constants

Done, can we resolve this conversation?

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.

src/taxonomy/tag-list/hooks.ts

jesperhodge · 2026-03-12T16:18:16Z

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.

Hi @bradenmacdonald , for now our current designs and acceptance criteria have been aligned on:

new rows / subrows appear at the top, out of order
the table waits for success before adding them, but doesn't refresh / re-sort the data from the backend, so that a workflow of adding multiple tags and subtags does not get interrupted
the newly created rows are highlighted (out of scope, there's a subtask).

src/taxonomy/tag-list/messages.ts

jesperhodge · 2026-03-17T20:09:23Z

Issue:
we noted that there are 4 levels of tags in a taxonomy, not 3. I will add a fix to that to this PR.

jesperhodge · 2026-03-17T20:29:06Z

Issue: we noted that there are 4 levels of tags in a taxonomy, not 3. I will add a fix to that to this PR.

Fixed, but please note that there is a backend bug that makes it so when you create a great-grandchild subtag and refresh, it disappears. That is out of scope and subject of openedx/modular-learning#257.

mgwozdz-unicon

Thank you for all your hard work on this @jesperhodge ! It looks good!

bradenmacdonald · 2026-03-20T02:01:25Z

Fixed, but please note that there is a backend bug that makes it so when you create a great-grandchild subtag and refresh, it disappears. That is out of scope and subject of openedx/modular-learning#257.

I replied there, but I'll mention it here too: it's not a bug; the API just allows you to load 3 levels at any one time, but you can still load children of depth 4 or more as long as you don't try loading them from the root. If you just call "get children" on the level 3 tags, you'll get the level 4 tags, and so on.

The initial API response also tells you if any of the level 3 tags have children that weren't included in the response due to this limitation.

bradenmacdonald

Partial review; I haven't quite gone through everything yet. I don't have any major concerns, and I'm excited to get this merged.

Two things stood out to me though: (1) as I mentioned, I am not convinced we need the "modes" you've implemented here, although I won't consider it a blocker, and (2) I think you'd be better off with a TagEditorContext or TagTreeContext that provides some shared data/state to all the components here instead of passing so many variables around as props. But again I won't consider that blocking.

Nothing with a "nit:" is a blocker either.

src/taxonomy/data/api.ts

src/taxonomy/data/apiHooks.ts

bradenmacdonald · 2026-03-20T20:47:12Z

src/taxonomy/tag-list/constants.ts

+const TABLE_MODES = {
+  VIEW: 'view',
+  DRAFT: 'draft',
+  PREVIEW: 'preview',
+};


Please document this, if you're keeping these modes. But as I mentioned in the other comment, I think we should put a TODO here that there's probably a way to simplify this.

What would you like me to write in the comments here? These are just enum-likes. The documentation I wrote is in the TagListTable component. I'm happy to add any comments you want here

Suggested change

const TABLE_MODES = {

VIEW: 'view',

DRAFT: 'draft',

PREVIEW: 'preview',

};

/** Tag list table modes - see explanation in `<TagListTable>` component (`src/taxonomy/tag-list/TagListTable.tsx`) */

const TABLE_MODES = {

VIEW: 'view',

DRAFT: 'draft',

PREVIEW: 'preview',

};

src/taxonomy/tag-list/tagTree.ts

bradenmacdonald · 2026-03-20T21:31:14Z

src/taxonomy/tag-list/tagTree.ts

+    }
+  }
+
+  private validateNoCycles(items: TagData[]) {


Why do we need cycle validation? Doesn't the backend already do that?

I want the tag tree to be very robust and hope to avoid any type of bugs that could originate with this data structure. So I don't want to rely solely on the backend - the data structure just makes sure it's valid when it's constructed, so if that validation fails at least there's a clear error indication.

Fine, please just mention that in the code, because it's not obvious.

Suggested change

private validateNoCycles(items: TagData[]) {

/** For extra robustness, we verify that there are no cycles in the data. (The backend also guarantees this.) */

private validateNoCycles(items: TagData[]) {

src/taxonomy/taxonomy-detail/constants.ts

bradenmacdonald · 2026-03-20T21:36:05Z

src/taxonomy/tree-table/EditableCell.tsx

+  onKeyDown?: (event: React.KeyboardEvent<HTMLInputElement>) => void;
+  onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;
+  errorMessage?: string;
+  isSaving?: boolean;


Nit: please add a /** comment */ explaining isSaving and autoFocus

Thanks! For future reference though, the @property... syntax you used is only required in plain JS files. In TypeScript files, you can just put the comments inline which is way easier. As you can see from the screneshot, VSCode still picks up the documentation and shows it everywhere those props are used.

src/taxonomy/tree-table/NestedRows.tsx

bradenmacdonald · 2026-03-25T22:34:22Z

src/taxonomy/tree-table/EditableCell.tsx

+/**
+ * Props for the EditableCell component.
+ * @interface EditableCellProps
+ * @property {string} [initialValue] - The initial value to display in the cell
+ * @property {function} [onKeyDown] - Callback function triggered on keyboard events
+ * @property {function} [onChange] - Callback function triggered when the input value changes
+ * @property {string} [errorMessage] - Error message to display if validation fails
+ * @property {boolean} [isSaving] - Indicates whether the cell value is currently being saved to the server
+ * @property {boolean} [autoFocus] - If true, the input field will automatically receive focus when the cell
+ * enters edit mode
+ * @property {function} [getInlineValidationMessage] - Function that returns a validation message to be displayed
+ * based on the current input value.
+ */
+interface EditableCellProps {
+  initialValue?: string;
+  onKeyDown?: (event: React.KeyboardEvent<HTMLInputElement>) => void;
+  onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;
+  errorMessage?: string;
+  isSaving?: boolean;
+  autoFocus?: boolean;
+  getInlineValidationMessage?: (value: string) => string;
+}


Suggested change

/**

* Props for the EditableCell component.

* @interface EditableCellProps

* @property {string} [initialValue] - The initial value to display in the cell

* @property {function} [onKeyDown] - Callback function triggered on keyboard events

* @property {function} [onChange] - Callback function triggered when the input value changes

* @property {string} [errorMessage] - Error message to display if validation fails

* @property {boolean} [isSaving] - Indicates whether the cell value is currently being saved to the server

* @property {boolean} [autoFocus] - If true, the input field will automatically receive focus when the cell

* enters edit mode

* @property {function} [getInlineValidationMessage] - Function that returns a validation message to be displayed

* based on the current input value.

*/

interface EditableCellProps {

initialValue?: string;

onKeyDown?: (event: React.KeyboardEvent<HTMLInputElement>) => void;

onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;

errorMessage?: string;

isSaving?: boolean;

autoFocus?: boolean;

getInlineValidationMessage?: (value: string) => string;

}

/**

* Props for the EditableCell component.

*/

interface EditableCellProps {

/** The initial value to display in the cell */

initialValue?: string;

/** Callback function triggered on keyboard events */

onKeyDown?: (event: React.KeyboardEvent<HTMLInputElement>) => void;

/** Callback function triggered when the input value changes */

onChange?: (event: React.ChangeEvent<HTMLInputElement>) => void;

/** Error message to display if validation fails */

errorMessage?: string;

/** Indicates whether the cell value is currently being saved to the server */

isSaving?: boolean;

/** If true, the input field will automatically receive focus when the cell enters edit mode */

autoFocus?: boolean;

/** Function that returns a validation message to be displayed based on the current input value. */

getInlineValidationMessage?: (value: string) => string;

}

bradenmacdonald · 2026-03-25T22:36:23Z

src/taxonomy/data/apiHooks.ts

+export const useCreateTag = (taxonomyId: number, intl?: any) => {
+  const queryClient = useQueryClient();


Nit:

Suggested change

export const useCreateTag = (taxonomyId: number, intl?: any) => {

const queryClient = useQueryClient();

export const useCreateTag = (taxonomyId: number) => {

const queryClient = useQueryClient();

const intl = useIntl();

You'd need to add the import above and and perhaps adjust the tests, but this is preferable to passing intl as a parameter - which you should almost never have to do.

bradenmacdonald · 2026-03-25T22:38:28Z

src/taxonomy/tag-list/constants.ts

+const TABLE_MODES = {
+  VIEW: 'view',
+  DRAFT: 'draft',
+  PREVIEW: 'preview',
+};


Suggested change

const TABLE_MODES = {

VIEW: 'view',

DRAFT: 'draft',

PREVIEW: 'preview',

};

/** Tag list table modes - see explanation in `<TagListTable>` component (`src/taxonomy/tag-list/TagListTable.tsx`) */

const TABLE_MODES = {

VIEW: 'view',

DRAFT: 'draft',

PREVIEW: 'preview',

};

bradenmacdonald · 2026-03-25T22:40:20Z

src/taxonomy/tag-list/TagListTable.tsx

+
+  const [creatingParentId, setCreatingParentId] = useState<RowId | null>(null);
+  const [editingRowId, setEditingRowId] = useState<RowId | null>(null);
+  const [toast, setToast] = useState({ show: false, message: '', variant: 'success' });


If you're not going to refactor this, please add least add a // TODO: change to use the global ToastContext

bradenmacdonald · 2026-03-25T22:44:30Z

src/taxonomy/tag-list/tagTree.ts

+    }
+  }
+
+  private validateNoCycles(items: TagData[]) {


Fine, please just mention that in the code, because it's not obvious.

Suggested change

private validateNoCycles(items: TagData[]) {

/** For extra robustness, we verify that there are no cycles in the data. (The backend also guarantees this.) */

private validateNoCycles(items: TagData[]) {

bradenmacdonald · 2026-03-25T22:45:03Z

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)) {


Fine, please just explain that in the code.

bradenmacdonald · 2026-03-25T22:46:23Z

src/taxonomy/tree-table/NestedRows.tsx

+ * @interface NestedRowsProps
+ * @property {TreeRow} parentRow - The parent row object from TanStack React Table
+ * @property {string} parentRowValue - The value identifier of the parent row
+ * @property {boolean} [isCreating] - Whether a new child row is currently being created for this parent
+ * @property {function} [onSaveNewChildRow] - Callback when a new child row is saved (receives value and parentRowValue)
+ * @property {function} [onCancelCreation] - Callback when child row creation is cancelled
+ * @property {TreeRow[]} [childRowsData] - Array of child row objects to render
+ * @property {number} [depth] - Current nesting depth level (used for indentation calculation)
+ * @property {string} [draftError] - Error message to display in draft creation form
+ * @property {function} [setDraftError] - Setter function for draft error state
+ * @property {RowId | null} [creatingParentId] - ID of the row currently in creation mode
+ * @property {function} [setCreatingParentId] - Setter function for which row is in creation mode
+ * @property {function} setIsCreatingTopRow - Callback to set whether top-level row creation is active
+ * @property {CreateRowMutationState} createRowMutation - State object for the row creation mutation
+ * (isPending, isError, error)
+ * @property {function} validate - Validation function for new row values
+ * (receives value and optional 'soft' or 'hard' mode; in 'hard' mode an exception is thrown on validation failure)


Same here - this JSDoc format is technically fine but it duplicates the type information that's already below, and there's a much simpler and more readable inline syntax you can use in TypeScript files.

src/taxonomy/tag-list/tagTree.ts

bradenmacdonald · 2026-03-25T22:55:54Z

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 guess I was hoping it would be much less complicated, and the only differences in behavior would be limited to the NestedRows component, but I'll defer to your opinion on it.

The main thing about lastCreatedTag is it doesn't have any global effect; it just affects the parent of the lastCreatedTag (which you'd store in the state), so that within the NestedRows component, you'd have a check that re-orders a local copy of its childRowsData if parentRowValue == lastCreatedTag.parentValue.

So I'm thinking it would just be a few lines of code in <NestedRows>, but maybe I am missing some other behavior that we need.

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 15 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

bradenmacdonald reviewed Mar 3, 2026

View reviewed changes

tbain reviewed Mar 4, 2026

View reviewed changes

src/taxonomy/tag-list/hooks.ts Show resolved Hide resolved

jesperhodge added 4 commits March 12, 2026 11:18

test: increase coverage

e614c16

fix: lint

1f1e366

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

1ff082f

test: coverage

dc2db61

jesperhodge marked this pull request as ready for review March 12, 2026 16:10

fix: tests

1cd7a44

mgwozdz-unicon reviewed Mar 13, 2026

View reviewed changes

src/taxonomy/tag-list/messages.ts Outdated Show resolved Hide resolved

jesperhodge requested review from bradenmacdonald and ormsbee March 16, 2026 15:34

fix: show correct number of taxonomy levels

9bdd5a4

jesperhodge added 2 commits March 17, 2026 16:57

refactor: extract constants

a3f8899

fix: PR comments

631d22d

mgwozdz-unicon approved these changes Mar 17, 2026

View reviewed changes

jesperhodge added 2 commits March 17, 2026 20:03

fix: tests

407835d

fix: url parameter breaking things

faea987

jesperhodge mentioned this pull request Mar 20, 2026

fix: return correct taxonomy depth up to max depth openedx/openedx-core#507

Closed

bradenmacdonald requested changes Mar 20, 2026

View reviewed changes

jesperhodge mentioned this pull request Mar 24, 2026

Rename taxonomy tags #2939

Open

kdmccormick requested review from kdmccormick and removed request for ormsbee March 24, 2026 20:18

jesperhodge added 2 commits March 25, 2026 17:19

fix: PR comments and cleanup

b3d25bd

fix: typo

6f2f6b9

jesperhodge requested a review from bradenmacdonald March 25, 2026 21:25

bradenmacdonald added the create-sandbox open-craft-grove should create a sandbox environment from this PR label Mar 25, 2026

fix: types

3a96b58

bradenmacdonald reviewed Mar 25, 2026

View reviewed changes

	private validateNoCycles(items: TagData[]) {
	/** For extra robustness, we verify that there are no cycles in the data. (The backend also guarantees this.) */
	private validateNoCycles(items: TagData[]) {

		export const useCreateTag = (taxonomyId: number, intl?: any) => {
		const queryClient = useQueryClient();

Conversation

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

Uh oh!

Description

Acceptance Criteria

Architecture

AI Usage

What is not in scope

Uh oh!

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

Uh oh!

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!

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!

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!

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!

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!

Uh oh!

jesperhodge commented Mar 12, 2026

Uh oh!

Uh oh!

jesperhodge commented Mar 17, 2026

Uh oh!

jesperhodge commented Mar 17, 2026

Uh oh!

mgwozdz-unicon left a comment

Choose a reason for hiding this comment

Uh oh!

jesperhodge commented Feb 12, 2026 •

edited

Loading

openedx-webhooks commented Feb 12, 2026 •

edited

Loading

bradenmacdonald commented Mar 20, 2026 •

edited

Loading