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

Convert project to typescript #33 #59

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

stavio12
Copy link
Contributor

  • converted project to typescript
  • changed default bg color to success

Copy link
Owner

@claeusdev claeusdev left a 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>
Copy link
Owner

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>)

Copy link
Contributor Author

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;

Copy link
Owner

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?

src/App.tsx Outdated Show resolved Hide resolved
@@ -1,13 +1,24 @@
import React from "react";
import { CSSTransition } from "react-transition-group";
// @ts-ignore
Copy link
Owner

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?

Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants