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

PROD-2481 Update manage datasets pages #5191

Merged
merged 34 commits into from
Aug 14, 2024

Conversation

lucanovera
Copy link
Contributor

@lucanovera lucanovera commented Aug 14, 2024

Description Of Changes

Complete refactor of every dataset page. Updated to use our new table component, breadcrumb navigation, add a table of collections.

Screenshots

Dataset List Page
Screen Shot 2024-08-14 at 02 03 53

Dataset Detail Page
Screen Shot 2024-08-14 at 02 03 56

Edit collection drawer
Screen Shot 2024-08-14 at 02 07 50

Delete confirmation modal
Screen Shot 2024-08-14 at 02 07 56

Collection detail page
Screen Shot 2024-08-14 at 02 08 07

Edit field drawer
Screen Shot 2024-08-14 at 02 08 10

Edit data category for field using picker
Screen Shot 2024-08-14 at 02 08 16

Code Changes

  • Updated Breadcrumbs component
  • Updated Edit drawers for datasets, collections and fields
  • Updated dataset detail page, now it list collections in a table
  • Added new page for listing all fields

Steps to Confirm

  • Log in to admin-ui
  • Add a new dataset using YAML or using Detection&Discovery features
  • Go to Data Inventory > Manage datasets
  • Check that the new UI matches the new designs (link found here https://ethyca.atlassian.net/browse/PROD-2481)
  • Check that the drawers work, and only fields display data categories for editing
  • Check that the search bar works
  • Check that the data category picker in the fields table works correctly

Pre-Merge Checklist

  • All CI Pipelines Succeeded
  • Issue Requirements are Met
  • Update CHANGELOG.md

Copy link

vercel bot commented Aug 14, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
fides-plus-nightly ✅ Ready (Inspect) Visit Preview 💬 Add feedback Aug 14, 2024 8:11pm

@lucanovera lucanovera changed the title Prod 2481 update manage datasets view PROD-2481 Update manage datasets pages Aug 14, 2024
Copy link
Contributor Author

@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.

Adding some comments in the code

@@ -1,13 +1,26 @@
import { Breadcrumb, BreadcrumbItem, BreadcrumbLink } from "fidesui";
import {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated the breadcrumbs component. Added support for icons, expanded Link to be able to receive query params, added prop optionally be able to override some styles. Remove isOpaque props since it was too specific and it won't be used anymore. Removed onClick since it wasn't working nor being used.

@@ -1,7 +1,7 @@
import {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated edit drawer component with some new styling.

@@ -0,0 +1,12 @@
import { createIcon } from "fidesui";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copied the icons from the D&D branch but I placed them in a more generic folder so we can use them for both features.

@@ -1,4 +1,4 @@
import { Box, BoxProps } from "fidesui";
import { Box, BoxProps, Flex } from "fidesui";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added rightContent prop to PageHeader. Useful in new designs for buttons on the right of the page header.

@@ -0,0 +1,103 @@
import {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Made a more generic version of the TaxonomyDisplayAndEdit component so I could use it in the dataset page. The idea is that later we can re-use it for D&D or for the TaxonomyDisplayAndEdit component.

@@ -0,0 +1,24 @@
/**
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Just a tiny wrapper for normal Breadcrumbs component with some added styles.

checked={checkedDataCategories}
onChecked={setCheckedDataCategories}
tooltip={DATASET.data_categories.tooltip}
variant="block"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Talked with jack about this. We're removing the data categories selector for datasets and collections, only fields should show this.

updateDataset(updatedDataset);
onClose();
}
const updatedField = { ...field!, ...values };
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am this could be simplified in the future. But here I'm reusing the existing helper functions that are unit tested.

Copy link
Contributor

@jpople jpople left a comment

Choose a reason for hiding this comment

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

Changes look good to me generally, but I found two small UI issues.

Icon sizes in the breadcrumbs are slightly inconsistent:

Screenshot 2024-08-14 at 10 20 39

Compare designs:

Screenshot 2024-08-14 at 10 23 32

Also, if I delete the provided data category and end up in a "None" state, there's no + button so I can't add a new category with the picker:

Screenshot 2024-08-14 at 10 21 44

breadcrumbs: {
title: string;
link?: string;
link?: Url; // Next.js link url. It can be a string or an URL object (accepts query params)
Copy link
Contributor

Choose a reason for hiding this comment

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

Didn't know this existed, neat.

@lucanovera
Copy link
Contributor Author

Changes look good to me generally, but I found two small UI issues.

Icon sizes in the breadcrumbs are slightly inconsistent:

Screenshot 2024-08-14 at 10 20 39

Compare designs:

Screenshot 2024-08-14 at 10 23 32

Also, if I delete the provided data category and end up in a "None" state, there's no + button so I can't add a new category with the picker:

Screenshot 2024-08-14 at 10 21 44

@jpople Thanks for the CR. Fixed the issues you mentioned.

Data category picker now has the + icon instead of saying None.
Captura de pantalla 2024-08-14 a la(s) 3 04 53 p  m

And the icon sizes are normalized now:
Captura de pantalla 2024-08-14 a la(s) 3 13 33 p  m

Copy link

cypress bot commented Aug 14, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 3333d47a87 ℹ️
Started Aug 14, 2024 8:21 PM
Ended Aug 14, 2024 8:21 PM
Duration 00:36 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

@lucanovera lucanovera merged commit 6926da5 into main Aug 14, 2024
13 checks passed
@lucanovera lucanovera deleted the PROD-2481-Update-manage-datasets-view branch August 14, 2024 20:22
Copy link

cypress bot commented Aug 14, 2024



Test summary

4 0 0 0


Run details

Project fides
Status Passed
Commit 6926da5
Started Aug 14, 2024 8:35 PM
Ended Aug 14, 2024 8:35 PM
Duration 00:37 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Cloud ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Cloud

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.

2 participants