Fix pending matches bug #1189
Fix pending matches bug #1189
Conversation
| @@ -756,7 +756,3 @@ hr { | |||
| .STATUS_PARSED:not(:hover) { | |||
| background: #fff !important; | |||
| } | |||
|
|
|||
| .STATUS_POTENTIAL_MATCH--ACTIONS + .STATUS_POTENTIAL_MATCH--ACTIONS { | |||
| display: none !important; | |||
There was a problem hiding this comment.
This line explicitly hid any confirm/reject buttons after the first.
| @@ -71,7 +71,7 @@ export default function FacilityListItemsDetailedTableRowCell({ | |||
| </div> | |||
| { | |||
| data.map((item, index) => ( | |||
| <Fragment key={hasActions ? item.id : item}> | |||
| <Fragment key={item.id ? item.id : item}> | |||
There was a problem hiding this comment.
This line was causing the main facility to receive an object as a key, instead of an id, causing a key warning in the console.
TaiWilkin
force-pushed
the
tw/fix-potential-match-bug
branch
from
055e72e
to
c049030
Compare
| <FacilityListItemsDetailedTableRowCell | ||
| title | ||
| title=" " |
There was a problem hiding this comment.
A string title is required by the FacilityListItemsDetailedTableRowCell. Not passing one causes a prop-type warning.
|
CI caught some lint |
|
Looking at this now. |
There was a problem hiding this comment.
Thanks for digging into one of the trickiest bits of code in the project and finding a relatively simple solution. I confirmed that this is working. Nice explanatory comments.
@designmatty I decided to get this partial fix to #1185 sooner rather than later because a significant feature is broken on production. I would like to follow up with a more comprehensive refactor that aligns each potential match.
@TaiWilkin could you please add a note explaining the challenge you encountered when trying to align the matches?
During confirm/reject analysis of a list, the following bug was noticed. When an entry had multiple potential matches, names and address of the multiple facilities were not broken by line items. The links were linked to an incorrect address. Additionally, the confirm and reject button showed only for the top-most potential match. When facilities have multiple potential matches, the matches show in individual line items with a Confirm and Reject option for each potential match. There were a minor propType warning and key warning which have also been corrected.
TaiWilkin
force-pushed
the
tw/fix-potential-match-bug
branch
from
c049030
to
864e4ae
Compare
Overview
During confirm/reject analysis of a list, the following bug was noticed.
When an entry had multiple potential matches, names and address of the
multiple facilities were not broken by line items. The links were
linked to an incorrect address. Additionally, the confirm and reject
button showed only for the top-most potential match.
When facilities have multiple potential matches, the matches show in
individual line items with a Confirm and Reject option for each
potential match.
There were a minor propType warning and key warning which have also
been corrected.
Connects #1185
Demo
Notes
The line items aren't currently perfectly aligned. This PR fixes the functionality issue, but additional styling might be desired in the future.
Currently, the matches are all stored in a single row, with each cell containing the section header and multiple matches' data for that value. Due to the varying height of the cells, and the cell component being used in multiple places, my attempts to fix this with styling were inefficient. I also worried that a set height to the cell content would cause overflow issues.
My suggested long-term fix for the styling is to refactor this section to render the headers and each facility match as separate rows in the table, which would keep the line-items aligned despite the variable height.
Testing Instructions
vagrant sshand./scripts/serverChecklist
fixup!commits have been squashed