Skip to content
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

[TS migration] Reafactor tooltips.js to use typescript #2073

Merged
merged 14 commits into from
Dec 17, 2023
Merged
6 changes: 3 additions & 3 deletions packages/desktop-client/src/components/NotesButton.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { remarkBreaks, sequentialNewlinesPlugin } from '../util/markdown';
import Button from './common/Button';
import Text from './common/Text';
import View from './common/View';
import { Tooltip, useTooltip } from './tooltips';
import { Tooltip, type TooltipPosition, useTooltip } from './tooltips';

const remarkPlugins = [sequentialNewlinesPlugin, remarkGfm, remarkBreaks];

Expand Down Expand Up @@ -86,7 +86,7 @@ const markdownStyles = css({
type NotesTooltipProps = {
editable?: boolean;
defaultNotes?: string;
position?: string;
position?: TooltipPosition;
onClose?: (notes: string) => void;
};
function NotesTooltip({
Expand Down Expand Up @@ -138,7 +138,7 @@ type NotesButtonProps = {
width?: number;
height?: number;
defaultColor?: string;
tooltipPosition?: string;
tooltipPosition?: TooltipPosition;
style?: CSSProperties;
};
export default function NotesButton({
Expand Down
Original file line number Diff line number Diff line change
@@ -1,18 +1,27 @@
import React, { Component, createContext, createRef, useState } from 'react';
import {
Component,
createContext,
createRef,
useState,
type RefObject,
type ReactNode,
type MouseEventHandler,
type MouseEvent,
} from 'react';
import ReactDOM from 'react-dom';

import { css } from 'glamor';

import { styles, theme } from '../style';
import { type CSSProperties, styles, theme } from '../style';

export const IntersectionBoundary = createContext();
export const IntersectionBoundary = createContext<RefObject<HTMLElement>>(null);

export function useTooltip() {
const [isOpen, setIsOpen] = useState(false);
const [isOpen, setIsOpen] = useState<boolean>(false);

return {
getOpenEvents: (events = {}) => ({
onClick: e => {
getOpenEvents: (events: { onClick?: MouseEventHandler } = {}) => ({
onClick: (e: MouseEvent) => {
e.stopPropagation();
events.onClick?.(e);
setIsOpen(true);
Expand All @@ -24,14 +33,51 @@ export function useTooltip() {
};
}

export class Tooltip extends Component {
export type TooltipPosition =
| 'top'
| 'top-left'
| 'top-right'
| 'bottom'
| 'bottom-left'
| 'bottom-right'
| 'bottom-stretch'
| 'top-stretch'
| 'bottom-center'
| 'top-center'
| 'left-center'
| 'right';

type TooltipProps = {
position?: TooltipPosition;
onClose?: () => void;
forceLayout?: boolean;
forceTop?: number;
ignoreBoundary?: boolean;
targetRect?: DOMRect;
offset?: number;
style?: CSSProperties;
width?: number;
children: ReactNode;
targetHeight?: number;
};
type MutableDomRect = {
top: number;
left: number;
width: number;
height: number;
};

export class Tooltip extends Component<TooltipProps> {
static contextType = IntersectionBoundary;
state = { position: null };
position: TooltipPosition;
contentRef: RefObject<HTMLDivElement>;
cleanup: () => void;
target: HTMLDivElement;

constructor(props) {
super(props);
this.position = props.position || 'bottom-right';
this.contentRef = createRef();
this.contentRef = createRef<HTMLDivElement>();
}

setup() {
Expand Down Expand Up @@ -111,7 +157,7 @@ export class Tooltip extends Component {
const { ignoreBoundary = false } = this.props;

if (!ignoreBoundary && this.context) {
return this.context.current;
return (this.context as RefObject<HTMLElement>).current;
}
MatissJanis marked this conversation as resolved.
Show resolved Hide resolved
return document.body;
}
Expand All @@ -123,11 +169,11 @@ export class Tooltip extends Component {

if (
container.parentNode &&
container.parentNode.style.overflow === 'auto'
(container.parentNode as HTMLElement).style.overflow === 'auto'
) {
return container.parentNode;
return container.parentNode as HTMLElement;
}
return container;
return container as HTMLElement;
}
IzStriker marked this conversation as resolved.
Show resolved Hide resolved

layout() {
Expand All @@ -138,9 +184,10 @@ export class Tooltip extends Component {
}

const box = contentEl.getBoundingClientRect();
const anchorEl = this.target.parentNode;
const anchorEl = this.target.parentNode as HTMLElement;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

❓ question: ‏same question here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this is necessary because .partentNode is of type ParentNode which you can't call getBoundingClientRect on.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might need to add a type guard here. That would solve the problem.

In my experience manual type coercions tend to lead to subtle bugs that are very hard to catch. Hence why I'm so nitpicky about them. Sorry about that.

let anchorRect = targetRect || anchorEl.getBoundingClientRect();
let anchorRect: MutableDomRect =
targetRect || anchorEl.getBoundingClientRect();

// Copy it so we can mutate it
anchorRect = {
Expand Down Expand Up @@ -233,7 +280,13 @@ export class Tooltip extends Component {
}

getStyleForPosition(position, boxRect, anchorRect, containerRect, offset) {
const style = {
const style: {
top?: string;
bottom?: string;
left?: string;
right?: string;
width?: string;
} = {
top: 'inherit',
IzStriker marked this conversation as resolved.
Show resolved Hide resolved
bottom: 'inherit',
left: 'inherit',
Expand Down
6 changes: 6 additions & 0 deletions upcoming-release-notes/2073.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
category: Enhancements
authors: [IzStriker]
---

Migrate tooltips.js to typescript
Loading