OBPIH-6020 Preference Type Modal#4441
OBPIH-6020 Preference Type Modal#4441kchelstowski merged 8 commits intofeature/product-supplier-list-redesignfrom
Conversation
| <> | ||
| <span | ||
| className={label?.className} | ||
| onClick={onCellClick} |
There was a problem hiding this comment.
here it doesn't matter, because the definition of your method is () => ..., but be aware that if you had an argument, it would be called with the e from span.
I mean that your current code evaluets to:
onClick={e => onCellClick(e)}
| </span> | ||
| <PreferenceTypeModal | ||
| productSupplierId={productSupplierId} | ||
| isOpen={!!preferenceTypeModalData.length} |
There was a problem hiding this comment.
I don't mind this, but in general the double negations are difficult to read - how about doing it as:
isOpen={Boolean(preferenceTypeModalData.length)}There was a problem hiding this comment.
I don't have any preference about that, so I can change it
| <span className="preference-type-location">Default: </span> | ||
| <span>{name}</span> | ||
| </p> | ||
| ), |
There was a problem hiding this comment.
returning HTML like that feels weird to me - if it would not be difficult to change, how about just storing the name and destinationParty.name in the list and then mapping it in the JSX output (85th line)?
If it is more complicated than I think, then I guess you can skip it.
| </div> | ||
| <div className="preference-type-modal-list-container"> | ||
| <div> | ||
| {mapPreferenceTypes(modalData).default} |
There was a problem hiding this comment.
let's assign the result of the mapPreferenceTypes(modalData) to a variable, because doing it this way, you run this method twice - for the first time here, and for the second time in the 85th line.
|
Additionally I think it would be an interesting improvement if you somehow stored already visited product sources' preference type modal. |
| </div> | ||
| <div className="preference-type-modal-list"> | ||
| {mappedPreferenceTypes?.preferenceTypes?.map((preferenceType) => ( | ||
| <p> |
There was a problem hiding this comment.
you forgot to add the key
* OBPIH-6020 Add opening preference type modal and basic structure of it * OBPIH-6020 Add mapping preference types * OBPIH-6020 Pass id to the modal component for redirecting * OBPIH-6020 Override global modal styles * OBPIH-6020 Add url for editing * OBPIH-6020 Add translation for modal header * OBPIH-6020 Changes after review * OBPIH-6020 Add key
2 participants
No description provided.