-
Notifications
You must be signed in to change notification settings - Fork 6
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
261 UI: Update All Domains & All Vulnerabilities Tables #291
261 UI: Update All Domains & All Vulnerabilities Tables #291
Conversation
- Built new table with MUI DataGrid - Built new fetchAllDomains callBack function in useDomainApi.ts - Pagination, filtering, and sorting are client side now.
- Removed the query paramters as they are not needed.
…domainsall-vulnerabilities-tables
- made PAGE_SIZE consistent accross all tables. It defaults to 15 rows now. - replaced react table with material-ui table - added custom sort for severity levels - server side pagination and filtering for Domains and Vulnerabilities tables - removed old code for react-table in Domains
- Added a status change dropdown to the vulnerabilities table - Added a return if domains or vulnerabilities are empty upon filtering. Prevents the table from rendering empty if there are no domains or vulnerabilities matching the filter
- Removed old React Table code - Removed any unused code - Added hyperlink for the domain name - Added endIcons to Vulnerability, Domain, and Status columns - Added domianId to row data object
- Removed unused import for set in from Domains.tsx - Removed unused import for sub from Vulnerabilities.tsx - Removed unused initialSortBy function in Vulnerabilities.tsx - Removed unused initialFilterBy function in Vulnerabilities.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great. Maybe changing 'testDomain' to something more recognizable would be best. Otherwise LGTM.
- Created a new interface for the TestVulnerability object.
…domainsall-vulnerabilities-tables
- Removed console.logs from Domains.tsx and Vulnerabilities.tsx
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I was under the impression that button took you to the domain in question. |
@hawkishpolicy - after our review, yes let's keep it in this code push as it is exsiting functionality. We can make an Issue to review at a later time. Thanks. Approving this PR. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
After UAT and Review - LGTM
- Added aria-labels to icons with no text in Domains and Vulnerabilities tables - Removed tab indexes from icons in Domains and Vulnerabilities tables so as not to interfere with tabbing through the table -
…domainsall-vulnerabilities-tables
…domainsall-vulnerabilities-tables
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Change testdomain to domain
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Replace old React Table based tables with the new MUI Data Grid X tables
🗣 Description
💭 Motivation and context
🧪 Testing
📷 Screenshots (if appropriate)
Uncomment this section if a screenshot is needed.
✅ Pre-approval checklist
to reflect the changes in this PR.
✅ Pre-merge checklist
✅ Post-merge checklist