Skip to content
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.

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

Taxonomy nested dropdown #910

Merged
merged 8 commits into from
Jul 20, 2022
Merged

Conversation

allisonking
Copy link
Contributor

@allisonking allisonking commented Jul 15, 2022

Closes #853

Code Changes

  • Refactors some of the data category checkbox related code to be more generic so that we can also pass it different types of taxonomy, which often have a similar nested structure. "Data subjects" were trickier since they don't have a parent_key: they are not nested. So I had to update transformTaxonomyEntityToNodes to be able to handle when a thing is not actually nested.
  • New nested AccordionTree component
  • Hover state action buttons
  • Tests

Steps to Confirm

  • Visit /taxonomy and click around the accordion

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Documentation Updated:
    • documentation complete, or draft/outline provided (tag docs-team to complete/review on this branch)
    • documentation issue created (tag docs-team to complete issue separately)
  • Issue Requirements are Met
  • Relevant Follow-Up Issues Created
  • Update CHANGELOG.md

Description Of Changes

Only thing outstanding is this bit of CSS I'm struggling with: #910 (comment)

Other than that, good to review!

Screen.Recording.2022-07-18.at.4.13.58.PM.mov

Base automatically changed from aking-852-scaffold-taxonomy to main July 18, 2022 14:31
@allisonking allisonking force-pushed the aking-853-taxonomy-nested-dropdown branch 2 times, most recently from cf65d86 to ac8ca2b Compare July 18, 2022 15:54
@ssangervasi ssangervasi self-requested a review July 18, 2022 18:16
@allisonking allisonking marked this pull request as ready for review July 18, 2022 20:27
@allisonking allisonking requested a review from a team July 18, 2022 20:27
@allisonking allisonking force-pushed the aking-853-taxonomy-nested-dropdown branch from c2a8663 to 71bf4e9 Compare July 19, 2022 14:51
@allisonking
Copy link
Contributor Author

ready for review now!

@ThomasLaPiana
Copy link
Contributor

@allisonking my only nit here is that I think a better UX would be to have an accordion sound play every single time this dropdown is used

@ThomasLaPiana
Copy link
Contributor

@allisonking I'm getting this error here, going to try nuking it and restarting:

[frontend 5/5] RUN npm run export:
#9 0.510
#9 0.510 > export
#9 0.510 > next build && next export
#9 0.510
#9 0.838 info  - Loaded env from /fides/clients/admin-ui/.env.production
#9 0.970 Attention: Next.js now collects completely anonymous telemetry regarding usage.
#9 0.970 This information is used to shape Next.js' roadmap and prioritize features.
#9 0.970 You can learn more, including how to opt-out if you'd not like to participate in this anonymous program, by visiting the following URL:
#9 0.970 https://nextjs.org/telemetry
#9 0.970
#9 1.007 info  - Checking validity of types...
#9 10.23 Failed to compile.
#9 10.23
#9 10.23 ./src/theme/components/button.ts:1:10
#9 10.23 Type error: Module '"@chakra-ui/react"' has no exported member 'ComponentStyleConfig'.
#9 10.23
#9 10.23 > 1 | import { ComponentStyleConfig } from "@chakra-ui/react";
#9 10.23     |          ^
#9 10.23   2 |
#9 10.23   3 | const theme: ComponentStyleConfig = {   
#9 10.23   4 |   variants: {
------
executor failed running [/bin/sh -c npm run export]: exit code: 1
nox > Command docker build --target=frontend --tag ethyca/fidesctl:local-ui . failed with exit code 1
nox > Session dev failed.

@ThomasLaPiana
Copy link
Contributor

got it fixed, just had to wipe out my node_modules and let it do the install again

Copy link
Contributor

@ThomasLaPiana ThomasLaPiana left a comment

Choose a reason for hiding this comment

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

Great change! The accordion has a great mouse feel, and i love how it fades in as it expands 🚀

@allisonking allisonking merged commit 0eac4cf into main Jul 20, 2022
@allisonking allisonking deleted the aking-853-taxonomy-nested-dropdown branch July 20, 2022 15:10
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.

Build nested dropdown component for taxonomies
4 participants