Migrate tooltip to ts #4524
Migrate tooltip to ts #4524
Conversation
|
Beep boop! 🤖 Hey @ngprnk, thanks for your PR! One of my human friends will review this PR and get back to you as soon as possible. Stay awesome! 😎 |
|
|
|
Deploy preview for hasura-docs ready! Built with commit 47605d4 |
|
|
||
| export type ToolTipProps = { | ||
| message: ToolTypeGeneratorProps; | ||
| placement: string; |
There was a problem hiding this comment.
Can we narrow this type?
| placement: string; | |
| placement: 'top' | 'right' | 'bottom' | 'left'; |
| export type ToolTipProps = { | ||
| message: ToolTypeGeneratorProps; | ||
| placement: string; | ||
| children: any; |
There was a problem hiding this comment.
If you're using React.FC typings for children are already there.
| children: any; |
| message: string; | ||
| }; | ||
|
|
||
| export const tooltipGenerator: React.FC<ToolTypeGeneratorProps> = ({ |
There was a problem hiding this comment.
tooltipGenerator was a function from string to JSX.Element. Could you revert this change?
| }; | ||
|
|
||
| export type ToolTipProps = { | ||
| message: ToolTypeGeneratorProps; |
There was a problem hiding this comment.
We want to pass message as a string.
| message: ToolTypeGeneratorProps; | |
| message: string; |
| @@ -4,6 +4,5 @@ import { space } from 'styled-system'; | |||
| export const StyledTooltip = styled.span` | |||
| display: inline-block; | |||
| position: relative; | |||
|
|
|||
| ${space} | |||
There was a problem hiding this comment.
We should provide typings for StyledTooltip which are:
- SpaceProps (imported from
styled-system) - HTML span element attributes
import styled from 'styled-components';
import { space, SpaceProps } from 'styled-system';
export interface StyledTooltipProps
extends SpaceProps,
React.ComponentPropsWithRef<'span'> {}
export const StyledTooltip = styled.span<SpaceProps>`
display: inline-block;
position: relative;
${space}
`;| return <Tooltip id={message}>{message}</Tooltip>; | ||
| }; | ||
|
|
||
| export type ToolTipProps = { |
There was a problem hiding this comment.
Please use interface that extends StyledTooltipProps.
export interface ToolTipProps extends StyledTooltipProps { ... }
|
Closing this PR due to lack of activity |
|
Beep boop! 🤖 Hey @ngprnk! Sorry that your PR wasn’t merged. Do take a look at any of the other open issues to see if you’d like to take something up! We’re around on Discord if you have any questions 😄 |
Description
Related issue #4314
Migrated tooltip component to typescript
Changelog
CHANGELOG.mdis updated with user-facing content relevant to this PR.Affected components
Related Issues
Catalog upgrade
Does this PR change Hasura Catalog version?
Metadata
Does this PR add a new Metadata feature?
run_sqlauto manages the new metadata through schema diffing?run_sqlauto manages the definitions of metadata on renaming?export_metadata/replace_metadatasupports the new metadata added?GraphQL
Breaking changes