-
Notifications
You must be signed in to change notification settings - Fork 0
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
Enhance System Items Table Filtering #1140
base: develop
Are you sure you want to change the base?
Enhance System Items Table Filtering #1140
Conversation
Codecov ReportAttention: Patch coverage is
✅ All tests successful. No failed tests found.
Additional details and impacted files@@ Coverage Diff @@
## develop #1140 +/- ##
===========================================
+ Coverage 98.97% 98.99% +0.01%
===========================================
Files 98 99 +1
Lines 17736 18063 +327
Branches 3014 3056 +42
===========================================
+ Hits 17554 17881 +327
Misses 178 178
Partials 4 4 ☔ View full report in Codecov by Sentry. |
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, just one minor change can you make the usage status column larger please
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.
One question.
Also note the failing test: seems to be at
inventory-management-system/src/systems/__snapshots__/systemItemsTable.component.test.tsx.snap
Line 241 in 1935ec3
style="--header-catalogueItem_name-size: 250; --col-catalogueItem_name-size: 250; --header-mrt_row_expand-size: 100; --col-mrt_row_expand-size: 100; --header-mrt_row_select-size: 60; --col-mrt_row_select-size: 60; --header-item_serial_number-size: 250; --col-item_serial_number-size: 250; --header-item_modified_time-size: 350; --col-item_modified_time-size: 350; --header-item_delivered_date-size: 350; --col-item_delivered_date-size: 350; --header-item_is_defective-size: 200; --col-item_is_defective-size: 200; --header-item_usage_status-size: 200; --col-item_usage_status-size: 200; --header-mrt_row_spacer-size: 40; --col-mrt_row_spacer-size: 40; --header-item_created_time-size: 350; --col-item_created_time-size: 350;" |
@@ -1873,7 +2183,7 @@ exports[`SystemItemsTable > SystemItemsTable (normal) > renders correctly 1`] = | |||
data-index="6" | |||
> | |||
<div | |||
aria-label="Tue Sep 05 2023 23:00:00 GMT+0000 (Coordinated Universal Time),Tue Sep 05 2023 23:00:00 GMT+0000 (Coordinated Universal Time)" | |||
aria-label=",Tue Sep 05 2023 23:00:00 GMT+0000 (Coordinated Universal Time)" |
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.
aria-label=",Tue Sep 05 2023 23:00:00 GMT+0000 (Coordinated Universal Time)" | |
aria-label="Tue Sep 05 2023 23:00:00 GMT+0000 (Coordinated Universal Time)" |
Should the comma be there?
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 don't expect it to be there, so I'm not sure why it does appear? It could be a follow on from the previous dates in separate divs; the snapshot might split up dates listed.
…e-system-items-table-filtering-#1039
filterVariant: COLUMN_FILTER_VARIANTS.string, | ||
filterFn: COLUMN_FILTER_FUNCTIONS.string, | ||
columnFilterModeOptions: COLUMN_FILTER_MODE_OPTIONS.string, |
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 needs to be done like usage status and system id
0288208
to
96b2db6
Compare
Description
Enter a description of the changes here
Testing instructions
Add a set up instructions describing how the reviewer should test the code
Agile board tracking
closes #1039