add tooltip to button #377
Conversation
✅ Deploy Preview for superb-tarsier-e2aa29 ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
✅ Deploy Preview for vigilant-albattani-df38ec ready!
To edit notification comments on pull requests, go to your Netlify site settings. |
Deploying with
|
| Latest commit: |
18fdfc8
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://ecae812c.gmx-interface.pages.dev |
| Branch Preview URL: | https://tooltip-to-button.gmx-interface.pages.dev |
| @@ -461,6 +463,35 @@ export default function PositionEditor(props) { | |||
| [DEPOSIT]: t`Deposit`, | |||
| [WITHDRAW]: t`Withdraw`, | |||
| }; | |||
| const ERROR_TOOLTIP_MSG = { | |||
There was a problem hiding this comment.
- Should we move this to ./constants too?
- Can there be a case where there is an error code that is not in the config? Maybe we need to define a default message for that?
There was a problem hiding this comment.
- Other messages can need access to token or position info. Like in the case of SwapBox here (). So I think we can let it be here for now. LMK you thoughts.
gmx-interface/src/components/Exchange/SwapBox.js
Line 1818 in 8db708a
- I have updated the condition which is responsible for rendering the Tooltip
src/components/Exchange/constants.ts
Outdated
| @@ -0,0 +1,11 @@ | |||
| export enum ErrorCodes { | |||
There was a problem hiding this comment.
We recently discussed with G, enums should be called in the singular for consistency: ErrorCodes -> ErrorCode
You can also capitalize field names - ErrorCodes.Buffer
src/components/Exchange/constants.ts
Outdated
| invalidLiqPrice = "INVALID_LIQ_PRICE", | ||
| } | ||
|
|
||
| export enum ErrorTypes { |
There was a problem hiding this comment.
For me personally ErrorType sounds like the type of the error and not how we show it
I would call it ErrorDisplayType - but that's as you like it)
src/components/Tooltip/Tooltip.tsx
Outdated
| if (intervalCloseRef.current) { | ||
| clearInterval(intervalCloseRef.current); | ||
| intervalCloseRef.current = null; | ||
| } | ||
| if (!intervalOpenRef.current) { | ||
| intervalOpenRef.current = setTimeout(() => { | ||
| intervalOpenRef.current = window.setTimeout(() => { |
There was a problem hiding this comment.
hmm why we need window here?
if typescript shows an error here it's strange because in other places we use just setTimeout (see lib/sleep.ts)
There was a problem hiding this comment.
The difference is assigning to a variable with type null | number. I will update type.
| className?: string; | ||
| disableHandleStyle?: boolean; | ||
| handleClassName?: string; | ||
| onDisabled?: boolean; |
There was a problem hiding this comment.
- mm, onDisabled sounds like an event handler callback
Not sure properly naming here, maybe isHandlerDisabled/disablePointerEvents?
-
I think comment should be here instead of Wrapper, it would be clearer for user of Tooltip component
// For onMouseLeave to work on disabled button mouseleave don't trigger on disabled inputs and button react-component/tooltip#18 (comment) -
I'm not sure we need a Wrapper component with so little functionality, Wrapper sounds like something global, not just a specific rare case handler. Maybe leaving a conditional div or className event is enough.
[x] Add tooltip to swapbox button for Pool Exceeded
[x] Add tooltip for invalid liquidation price.
[x] Refactor Tooltip and TooltipWithPortal to tsx