Add Lilypad Tooltip component to existing tool bar buttons #6058
Conversation
| @@ -7,8 +7,7 @@ | |||
| "avatar-page.select": "Seleccionar", | |||
There was a problem hiding this comment.
You can ignore the language files. These are just formatting updates from my attempt to add translations, which have been removed
| @@ -50,6 +51,7 @@ export const ToolbarButton = forwardRef( | |||
| )} | |||
| disabled={disabled} | |||
| title={title} | |||
| onClick={onClick} | |||
There was a problem hiding this comment.
Not in this PR but as the FE matures it might be good to remove these {...rest} catch alls, side note I know they where there, I just think they can be cleaver yet misleading, you are establishing these nice prototypes soon to be TS type and I think it would be nice to have concrete props being sent to the component with no wild card catch all.
| defaultMessage: "Open the chat sidebar (T)" | ||
| }); | ||
|
|
||
| interface ChatToolbarButtonProps extends RefAttributes<any> { |
There was a problem hiding this comment.
We usually opt in to using types before interfaces. type ChatToolbarButtonPropsT = {}
This other suggestion is stylistic, we do it in the subscription team but we end interface,type, and enum with the first initial of what it is. CategoriesE, ButtonsI ,ChatToolbarButtonPropsT etc.
I don't think extends RefAttributes is being used here?
Screen.Recording.2023-05-01.at.4.56.20.PM.mov