-
Notifications
You must be signed in to change notification settings - Fork 6
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
Convert project to typescript #33 #59
base: master
Are you sure you want to change the base?
Conversation
stavio12
commented
Sep 12, 2022
- converted project to typescript
- changed default bg color to success
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good so far but just tiny improvements we could do to this.
src/App.tsx
Outdated
<option value="tcenter">Top Center</option> | ||
<option value="bleft">Bottom Left</option> | ||
<option value="bright">Bottom Right</option> | ||
<option value="bcenter">Bottom Center</option> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we can make this seven simpler by putting these values in an enum
or list and then looping through to display.
const SELECT_VALUES = [
{
title: "Default",
value: "left"
}
]
Then, we can do:
SELECT_VALUES.map(value => <option value={value}>{title}</option>)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
import React from "react";
import styles from ".././App.module.css";
import { Position, PositionSelectorProps } from "../utils/types";
const PositionSelector = ({ setPosition, position }: PositionSelectorProps) => {
const positions = [
{ value: "default", title: "Default" },
{ value: "tleft", title: "Top Left" },
{ value: "tright", title: "Top Right" },
{ value: "tcenter", title: "Top Center" },
{ value: "bleft", title: "Bottom left" },
{ value: "bright", title: "Bottom Right" },
{ value: "bcenter", title: "Bottom Center" },
];
return (
<>
{" "}
Position:
<select
id="position"
onChange={({ target }) =>
setPosition(target.value as unknown as Position)
}
value={position}
className={styles.dropdown}
>
{positions.map(
({ value, title }: { value: string; title: string }) => (
{title}
)
)}
</>
);
};
export default PositionSelector;
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very hard to read te code. could you format it?
@@ -1,13 +1,24 @@ | |||
import React from "react"; | |||
import { CSSTransition } from "react-transition-group"; | |||
// @ts-ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ignoring ts warnings or errors doesn't give us much confidence in our code, does it?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(alias) class CSSTransition<Ref extends HTMLElement | undefined>
import CSSTransition
'CSSTransition' cannot be used as a JSX component.
Its instance type 'CSSTransition<HTMLElement | undefined>' is not a valid JSX element.
The types returned by 'render()' are incompatible between these types.
Type 'import("/Users/stavio/Documents/Projects/Production/react-flash-message/node_modules/@types/testing-library__react/node_modules/@types/react/index").ReactNode' is not assignable to type 'React.ReactNode'.
Type '{}' is not assignable to type 'ReactNode'.ts
this is the error i am getting dont seem to find a solution to it