Skip to content

Code Review - #2

Open
jurecki wants to merge 17 commits into
code-reviewfrom
main
Open

Code Review#2
jurecki wants to merge 17 commits into
code-reviewfrom
main

Conversation

@jurecki

@jurecki jurecki commented Dec 20, 2020

Copy link
Copy Markdown
Owner

No description provided.

Comment thread index.html
<label for='subject' class='form__label'>Subject</label>
</div>

<textarea rows=12 class='form__textarea' placeholder='Your Meassage Here....' id='description' autocomplete="off" required>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Strasznie dziwnie działa rozszerzenie tego textarea. Nie wiem jak to osiągnąłeś, ale dobrze byłoby to poprawić :)
image

Comment thread index.html

<meta name="description" content="myPorfolio">
<link rel="preconnect" href="https://fonts.gstatic.com">
<!-- <link href="https://fonts.googleapis.com/css2?family=Russo+One&display=swap" rel="stylesheet"> -->

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nie powinno się zostawiać takich zakomentowanych części kodu.
Po to właśnie jest system kontroli wersji - Git. Jeśli dobrze się nim posługujesz i opisujesz commity, to wtedy z łatwością znajdziesz zmainę - jeśli będziesz chciał do czegoś wrócić. A tak to nie wiadomo po co ten kod jest i czy w ogóle będzie działa jak się go odkomentuje.

Comment thread README.md
Przygotowane projekty interfejsu użytkownika są tutaj po to, żebyś bez problemów postawił swoje pierwsze kroki w praktycznym wykorzystaniu HTML i CSS.
Jednakże zachęcamy do wykonania czegoś, co ukaże Twoje wnętrze :)
Dobrej zabawy!
## Table of contents

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Brakuje najważniejszej rzeczy czyli WIELKIEGO WIDOCZNEGO LINKU DO DEMO.
Dodatkowo przydałby się screenshot - zawsze to robi dobre wrażenie na wejściu.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dobra, jest - ale to, że nie zauważyłem na początku świadczy, że powinen być bardziej widoczny.

Comment thread index.html
<div class='form__group'>
<input type='text' class='form__input' placeholder='Full Name' id='name' autocomplete="off" required>
<label for='name' class='form__label'>Full Name</label>
<input type='email' class='form__input' placeholder='Email adress' id='email' autocomplete="off" required>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Dlaczego jest wyłączony autocomplete :) ?

Comment thread sass/base/_base.scss
font-size: 56.25%; // 1 rem = 9px, 9/16 = 56.25%
}

@include respond(tab-port) { //width < 900px

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Te komentarze mogą kłamać :) Lepiej na nich nie polegać i wywalić. Jaki tutaj miałeś problem, że wymagało to komentarza? //width < 900px

Comment thread sass/utils/_mixins.scss
@mixin respond($breakpoint) {

@if $breakpoint == phone-small {
@media only screen and (max-width: 22.5em) { @content }; //360px

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Czy ja się mylę, czy to niezbyt zadziała? Będziesz miał ustawienia responsywności zależne od rozmiaru fontów, a powinny być zależne od rozmiarów ekranu. Może dlatego masz bardzo małe literki :) ?

image

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