PSP-9710, PSP-9313: UI/UX Cleanup - Make icons consistent across PIMS #4541
Conversation
stairaku
commented
Dec 18, 2024
stairaku
commented
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4541 |
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4541 |
| return ( | ||
| <StyledRemoveIconButton | ||
| variant="primary" | ||
| title={title ?? 'edit'} |
There was a problem hiding this comment.
default title for remove buttons should be "remove" - not "edit"
| onClick: () => void; | ||
| title?: string; | ||
| icon?: React.ReactNode; | ||
| dataTestId?: string | null; |
There was a problem hiding this comment.
We have an existing pattern for passing data-testId to components without having to use a new property name.
See SectionListHeader.tsx for reference below:
There was a problem hiding this comment.
Thank you for the observation. Changing the data-testId pattern on both RemoveButton and ViewButton files.
| }) => ( | ||
| <StyledIconButton | ||
| variant="primary" | ||
| title={title ?? 'edit'} |
There was a problem hiding this comment.
View button default title should not be "edit" - maybe "view" would be more suitable?
| <StyledCompensationContainer> | ||
| {asCurrency ? formatMoney(Number(value)) : value} | ||
| <EditButton onClick={() => setIsEditing(true)} /> | ||
| </StyledCompensationContainer> |
There was a problem hiding this comment.
nit: This component is generic so the styled container shouldn't be named "StyledCompensationContainer" - maybe just "StyledContainer"? This is very minor of course
| )} | ||
|
|
||
| {hasClaim(Claims.CONTACT_VIEW) && ( | ||
| <StyledIconButton | ||
| <ViewButton |
There was a problem hiding this comment.
thanks for making these!
| </StyledRemoveLinkButton> | ||
| )} | ||
| <Col xs={1} className="p-0"> | ||
| {' '} |
There was a problem hiding this comment.
no sure whether we need this here?
There was a problem hiding this comment.
good catch! I forgot to clean that up!
|
See CodeCov Report Here: https://app.codecov.io/github/bcgov/psp/pull/4541 |
