Conversation
|
Another thought: could we also close the picker directly when a date is selected? |
|
I also noticed that the date and time inputs don't have the same height 🫣 
|
|
|
@adessy Fixed timeInput height and made the date picker close automatically after selecting a date ✅ |
| )} | ||
| {!apiSendErrors && hasNoParticipants && ( | ||
| <Box mb="8px"> | ||
| <Error text={formatMessage(errorMessages.no_recipients)} /> |
There was a problem hiding this comment.
I'm not sure the Error component is the right fit here, but @IvaKop would know better than me.
There was a problem hiding this comment.
It is ok to use this component with text instead of passing the apiErrors prop in this case.
IvaKop
left a comment
IvaKop
left a comment
There was a problem hiding this comment.
Some minor changes required but overall - looking good 👍
| > | ||
| {format(selectedTime, 'p', { locale: getLocale(locale) })} | ||
| </Button> | ||
| <InputContainer onClick={() => setVisible(true)}> |
There was a problem hiding this comment.
Avoid adding onClick handlers on containers. It is supposed to be added only to clickable elements like buttons.
There was a problem hiding this comment.
InputContainer is actually implemented as a styled button, so it is already a clickable element and intended to be interactive.
front/app/component-library/utils/containers/InputContainer.tsx
| )} | ||
| {!apiSendErrors && hasNoParticipants && ( | ||
| <Box mb="8px"> | ||
| <Error text={formatMessage(errorMessages.no_recipients)} /> |
There was a problem hiding this comment.
It is ok to use this component with text instead of passing the apiErrors prop in this case.
| import T from 'components/T'; | ||
| import ButtonWithLink from 'components/UI/ButtonWithLink'; | ||
| import Error from 'components/UI/Error'; | ||
| import errorMessages from 'components/UI/Error/messages'; |
There was a problem hiding this comment.
You are not supposed to import the error messages in this way. These are cenrtalized and retrieved by key.
Add this copy to the regular messages file of this component, instead.
IvaKop
left a comment
IvaKop
left a comment
There was a problem hiding this comment.
One small final comments about the error messages copy but other than that this is good to go 🚀
|
Due to Weave merge issue had to close this PR and open new branch #13691 |
4 participants
Email Scheduling Improvements
Changelog
For translators