Skip to content

New review branch#3

Open
kovesdieszter wants to merge 52 commits intoreview2from
main
Open

New review branch#3
kovesdieszter wants to merge 52 commits intoreview2from
main

Conversation

@kovesdieszter
Copy link
Copy Markdown
Owner

No description provided.

Copy link
Copy Markdown

@salierri salierri left a comment

Choose a reason for hiding this comment

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

Első TS projektnek igazán szép kód!

Comment thread simpletodo/src/App.tsx Outdated
</p>
</header>
<div>
<Title />
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Javascriptben a 2 space az elfogadott konvenció.
Vannak általánosan elfogadott style guideok (https://github.com/airbnb/javascript), ezeket jó ismerni és ezalapján formázni a kódot, akkor egységesebb lesz a többi js fejlesztővel.

Még jobb, ha be van húzva az ESlint / TSlint, és be vannak állítva szabályok amire szól.

Comment thread simpletodo/src/App.css Outdated
Comment thread simpletodo/src/components/CardForTask.tsx Outdated
Comment thread simpletodo/src/components/CardForTask.tsx Outdated
Comment thread simpletodo/src/components/CardForTask.tsx Outdated
Comment thread simpletodo/src/index.tsx
document.getElementById('root') as HTMLElement
);
root.render(
<React.StrictMode>
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Van valami oka hogy nem szeretnéd a strict mode-ot?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Nem most, de korábban problémát okozott a plusz 2 felesleges renderelés.

Comment thread README.md Outdated

- Clone the repository using the command git clone `https://github.com/kovesdieszter/SimpleToDo.git`.
- Install all necessary packages asked by your IDE.
- Run `npm start` from `simpletodo` directory.
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Fölösleges a projektnek még egy almappa, annak csak akkor van szerepe, ha több projektet is tárolsz egy repo-ban, ilyenkor mehet csak simán a gyökérbe.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Ez a komplikált deploy miatt most kivételesen már így maradna.

Comment thread simpletodo/src/App.css Outdated
Comment thread simpletodo/src/components/Container.tsx Outdated
Comment thread simpletodo/src/Card.css
YEAR: 'rgba(51, 170, 51, .3)'
}

function setBackgroundColor() {
Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This also works as a one-liner with arrow expression

Copy link
Copy Markdown

@salierri salierri 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 now!

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