-
Notifications
You must be signed in to change notification settings - Fork 24
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
PSP-9710, PSP-9313: UI/UX Cleanup - Make icons consistent across PIMS #4541
Changes from 6 commits
834e092
8596065
fd26b49
189f5e8
a1d974d
aaf3c43
4a93957
0b97e70
e552cf1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
import { ButtonProps } from 'react-bootstrap/Button'; | ||
import { FaEye } from 'react-icons/fa'; | ||
import { CSSProperties } from 'styled-components'; | ||
|
||
import { StyledIconButton } from './IconButton'; | ||
|
||
interface IViewButtonProps extends ButtonProps { | ||
onClick: () => void; | ||
title?: string; | ||
icon?: React.ReactNode; | ||
dataTestId?: string | null; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thank you for the observation. Changing the data-testId pattern on both RemoveButton and ViewButton files. |
||
style?: CSSProperties | null; | ||
} | ||
|
||
export const ViewButton: React.FunctionComponent<React.PropsWithChildren<IViewButtonProps>> = ({ | ||
onClick, | ||
title, | ||
icon, | ||
dataTestId, | ||
style, | ||
}) => ( | ||
<StyledIconButton | ||
variant="primary" | ||
title={title ?? 'edit'} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. View button default title should not be "edit" - maybe "view" would be more suitable? |
||
onClick={onClick} | ||
data-testid={dataTestId ?? 'view-button'} | ||
style={style} | ||
> | ||
{icon ?? <FaEye size={22} />} | ||
</StyledIconButton> | ||
); | ||
|
||
export default ViewButton; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,10 +2,11 @@ import React from 'react'; | |
import { Button, Form, Spinner } from 'react-bootstrap'; | ||
import { FaCheck } from 'react-icons/fa'; | ||
import NumberFormat from 'react-number-format'; | ||
import styled from 'styled-components'; | ||
|
||
import { formatMoney } from '@/utils'; | ||
|
||
import EditButton from '../../EditButton'; | ||
import EditButton from '../../buttons/EditButton'; | ||
|
||
/** | ||
* Non formik form that allows a user to toggle between a value and an input and save the input. | ||
|
@@ -65,9 +66,20 @@ export const ToggleSaveInputView: React.FC<IToggleSaveInputViewProps> = ({ | |
} else { | ||
return ( | ||
<> | ||
{asCurrency ? formatMoney(Number(value)) : value} | ||
<EditButton onClick={() => setIsEditing(true)} /> | ||
<StyledCompensationContainer> | ||
{asCurrency ? formatMoney(Number(value)) : value} | ||
<EditButton onClick={() => setIsEditing(true)} /> | ||
</StyledCompensationContainer> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nit: This component is generic so the styled container shouldn't be named "StyledCompensationContainer" - maybe just "StyledContainer"? This is very minor of course |
||
</> | ||
); | ||
} | ||
}; | ||
|
||
export const StyledCompensationContainer = styled.div` | ||
display: flex; | ||
flex-direction: row; | ||
justify-content: flex-end; | ||
align-items: center; | ||
margin-bottom: 0.5rem; | ||
align-items: center; | ||
`; |
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.
default title for remove buttons should be "remove" - not "edit"