Skip to content

Taxonomy "Save" button fix #7318

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
jpople merged 4 commits into from
Feb 9, 2026
Merged

Taxonomy "Save" button fix #7318

jpople merged 4 commits into from
Feb 9, 2026

Conversation

@jpople
Copy link
Contributor

@jpople jpople commented Feb 5, 2026 •
edited by atlassian bot
Loading

Ticket ENG-2483

Description Of Changes

No longer shows the "Save" button in the details drawer when editing core taxonomies since they can't be edited.

Steps to Confirm

  1. Create a custom taxonomy
  2. After clicking on the taxonomy root item, should be able to edit name and description from inside the drawer
  3. View a core taxonomy
  4. After clicking on the taxonomy root, name and description should be static and "Save" and "Delete" buttons shouldn't be shown

Code Changes

  • Update display logic for save button
  • Rename some files/components to clarify purpose and reflect the fact that the taxonomy edit drawer is no longer exclusive to custom taxonomies:
    • TaxonomyEditDrawer -> TaxonomyItemEditDrawer (had previously changed the component name but not the filename)
    • CustomTaxonomyEditDrawer -> TaxonomyEditDrawer
    • CustomTaxonomyDetails -> TaxonomyDetails

Pre-Merge Checklist

  • Issue requirements met
  • All CI pipelines succeeded
  • CHANGELOG.md updated
    • Add a db-migration This indicates that a change includes a database migration label to the entry if your change includes a DB migration
    • Add a high-risk This issue suggests changes that have a high-probability of breaking existing code label to the entry if your change includes a high-risk change (i.e. potential for performance impact or unexpected regression) that should be flagged
    • Updates unreleased work already in Changelog, no new entry necessary
  • UX feedback:
    • All UX related changes have been reviewed by a designer
    • No UX review needed
  • Followup issues:
    • Followup issues created
    • No followup issues
  • Database migrations:
    • Ensure that your downrev is up to date with the latest revision on main
    • Ensure that your downgrade() migration is correct and works
      • If a downgrade migration is not possible for this change, please call this out in the PR description!
    • No migrations
  • Documentation:
    • Documentation complete, PR opened in fidesdocs
    • Documentation issue created in fidesdocs
    • If there are any new client scopes created as part of the pull request, remember to update public-facing documentation that references our scope registry
    • No documentation updates required

@jpople jpople requested a review from a team as a code owner February 5, 2026 15:01
@jpople jpople requested review from lucanovera and removed request for a team February 5, 2026 15:01
@vercel
Copy link
Contributor

vercel bot commented Feb 5, 2026 •
edited
Loading

The latest updates on your projects. Learn more about Vercel for GitHub.

2 Skipped Deployments
Project Deployment Actions Updated (UTC)
fides-plus-nightly Ignored Ignored Preview Feb 5, 2026 3:20pm
fides-privacy-center Ignored Ignored Feb 5, 2026 3:20pm

Request Review

@greptile-apps
Copy link
Contributor

greptile-apps bot commented Feb 5, 2026

Greptile Overview

Greptile Summary

This PR updates the taxonomy edit drawer so core (non-custom) taxonomies no longer show the Save/Delete action footer, aligning the UI with the fact that core taxonomies are not editable. The change is localized to CustomTaxonomyEditDrawer.tsx, conditioning the DetailsDrawer footer on whether the taxonomy’s fides_key is part of TaxonomyTypeEnum.

Confidence Score: 4/5

  • This PR is likely safe to merge, with one UI behavior/type edge case to verify.
  • The change is small and localized to conditional rendering of the drawer footer; main risk is passing a boolean as footer if the DetailsDrawer API expects null/undefined or always renders a footer container.
  • clients/admin-ui/src/features/taxonomy/components/CustomTaxonomyEditDrawer.tsx

Important Files Changed

Filename Overview
clients/admin-ui/src/features/taxonomy/components/CustomTaxonomyEditDrawer.tsx Hide drawer footer actions (Save/Delete) for core taxonomies by conditioning the DetailsDrawer footer; adds onClose prop to avoid passing through DetailsDrawerProps.onClose.

greptile-apps[bot]
greptile-apps bot reviewed Feb 5, 2026
View reviewed changes
Copy link
Contributor

@greptile-apps greptile-apps bot left a comment

Choose a reason for hiding this comment

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

1 file reviewed, 1 comment

Edit Code Review Agent Settings | Greptile

clients/admin-ui/src/features/taxonomy/components/CustomTaxonomyEditDrawer.tsx Outdated
Comment on lines 77 to 92
destroyOnHidden
footer={
<Flex justify="space-between" className="w-full">
{isCustom && <Button onClick={onDelete}>Delete</Button>}
<Button
type="primary"
htmlType="submit"
form={FORM_ID}
loading={isUpdating}
>
Save
</Button>
</Flex>
isCustom && (
<Flex justify="space-between" className="w-full">
<Button onClick={onDelete}>Delete</Button>
<Button
type="primary"
htmlType="submit"
form={FORM_ID}
loading={isUpdating}
>
Save
</Button>
</Flex>
)
}
Copy link
Contributor

@greptile-apps greptile-apps bot Feb 5, 2026

Choose a reason for hiding this comment

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

Hidden footer breaks layout

footer={isCustom && (...)} passes false to DetailsDrawer for core taxonomies. If DetailsDrawer expects a ReactNode (and may render the footer container regardless), passing a boolean can render nothing but still reserve footer spacing or break conditional rendering logic. Prefer returning null explicitly: footer={isCustom ? (...) : null} so core taxonomies have no footer without type/behavior ambiguity.

Copy link
Contributor Author

@jpople jpople Feb 5, 2026

Choose a reason for hiding this comment

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

false works fine here, the footer container is not rendered and layout is unaffected.

@jpople jpople force-pushed the jpople/eng-2483/core-taxonomy-save-button branch from 6d89eb9 to fdd6f42 Compare February 5, 2026 15:09
jpople
jpople commented Feb 5, 2026
View reviewed changes
clients/admin-ui/src/features/taxonomy/components/TaxonomyEditDrawer.tsx
>
Save
</Button>
</Flex>
Copy link
Contributor Author

@jpople jpople Feb 5, 2026

Choose a reason for hiding this comment

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

This diff is confusing because of the rename, but the actual change is just here-- previously the "Save" button was outside the conditional block, just moved it inside.

lucanovera
lucanovera approved these changes Feb 9, 2026
View reviewed changes
Copy link
Contributor

@lucanovera lucanovera left a comment

Choose a reason for hiding this comment

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

Save button is no longer visible for core taxonomies as expected. Code changes look good. Approved

@jpople jpople added this pull request to the merge queue Feb 9, 2026
Merged via the queue into main with commit a0a634b Feb 9, 2026
46 checks passed
@jpople jpople deleted the jpople/eng-2483/core-taxonomy-save-button branch February 9, 2026 20:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@lucanovera lucanovera lucanovera approved these changes

+1 more reviewer

@greptile-apps greptile-apps[bot] greptile-apps[bot] left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@jpople @lucanovera