Develop by IamMK · Pull Request #2 · D0man/Academy

IamMK · 2019-01-16T19:52:38Z

Wszelkie informacje dodatkowe ode mnie podane w pliku README.md

D0man

Code review, PHP nie patrzyłem i rzeczy które są mniej oczywiste też nie
, ogólnie całkiem spoko ci poszło po za nazwami klas cssow i czasem nadmiernej strukturze w htmlu.

D0man · 2019-01-17T19:14:54Z

README.md

 # Academy
 Projekty wykonywane przez społeczność discorda
-[helloroman](https://discordapp.com/invite/VTyJc9N)  
+<<<<<<< HEAD


Head wskazuje na konflikty które nie rozwiazales, generalnie staraj się zakładać nowe readme dla swojego challengach jak chcesz je pullowac bo w ogólnym będzie no bardziej ogolnie

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

D0man · 2019-01-17T19:20:26Z

index.php

+</head>
+<body>
+    <div class="container">
+        <div class="container__coming-soon">


Potrzebujesz tego diva? Możesz dać tą klasę na figure, zmieniłbym też nazwę klasy bo to jest niezależny element od containera.

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

D0man · 2019-01-17T19:21:43Z

index.php

+        <div class="container__coming-soon">
+            <figure><img src="assets/coming-soon-ribbon.png" alt="Coming Soon"></figure>
+        </div>
+            <header class="header">


Wybacz, niedopatrzenie ;)

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

D0man · 2019-01-17T19:28:29Z

index.php

+                    <h1>CODE ACADEMY</h1>
+                </header>
+                <main class="main">
+                    <section class="main__form">


Jeżeli masz tylko jedna sekcję w main to nie ma potrzeby jej wypisywania zostawiasz po prostu main, nie dawaj też sekcji klasy form to mylące bo to formularz nie jest tylko sekcja który go zawiera

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

D0man · 2019-01-17T19:36:03Z

index.php

+                    <section class="main__form">
+                        <h2>Leave your email here</h2>
+                        <form action="backend/mailPushing.php" method="post">
+                            <div class="main__form--flex-column">


Potrzebujesz tego diva? Nie mógłby ostylowac po formie wtedy też nazwienictwo klas cssow u by się bardziej kupy trzymało, jeżeli starasz się w bramie pisać to staraj się formy trzymać w oddzielnym bloku bo nie zawsze będą w mainie, flex column na pierwszy rzut oka też nie wygląda jak modyfikator a jak element layoutu i powinien być oddzielna klasa, ogólnie strasznie długie nazwy klas masz nie wystarczyłby form lub form-email żeby było jaśniej

Poprawione, jednak to z całą świadomością pozostawiam do ponownej oceny - nie jestem pewien nazw.
Owy flex--column służy jako pośrednik w uzyskaniu napisu "Invalid email".

D0man · 2019-01-17T19:37:35Z

index.php

+                        <h2>Leave your email here</h2>
+                        <form action="backend/mailPushing.php" method="post">
+                            <div class="main__form--flex-column">
+                                <label for="mail"><?php if(isset($invalidEmail)) echo $invalidEmail['errorMessage'] ?></label>


Przy tym jak pisałem że bonus za rozwiązanie bez jsa bardziej chodziło mi o wersję cssow tego, ale fakt o phpie nie wspomniałem sprytnie to obraziłeś.

Owy php już nie istnieje. Użycie ciastek by nie działało w deep web ;)

D0man · 2019-01-17T19:39:08Z

stylesheets/main.css

@@ -0,0 +1,140 @@
+@import url('https://fonts.googleapis.com/css?family=Lato:300');


Lepiej nie importować czcionek za pomocą cssa

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

D0man · 2019-01-17T19:42:31Z

stylesheets/main.css

+    top: 0;
+}
+
+.container__coming-soon figure{


Staraj się nie stylowac po tagach tym bardziej że możesz to połączyć z klasą wyżej w ogóle dość często powtarzasz margin 0 może lepiej to dać w * i stworzyć Simple reset

Z jednej strony lepiej. Jednak strona była tworzona na defaultowych marginach i paddingach, które mają dużą tutaj przewagę. Póki co zostawiam.

Co do stylowania po tagach poprawione ;)

D0man · 2019-01-17T19:46:24Z

stylesheets/main.css

+
+.header figure{
+    /* width: 100vw; */
+    object-fit: cover;


To jest potrzebne?

Poprawione. Wiadomość wysyłam w celu zapamiętania co do poprawy :)

mateuszszmytko · 2019-01-19T20:37:40Z

Zazwyczaj nie pisze się słów wielkimi literami (CODE ACADEMY), użyj text-transform: uppercase tak by tylko wizualnie były wielkie.
BEMa używaj tylko do wiązania elementów, które są częścią jednej całości. Czyli np. .header__logo nie powinno być zależne od .header. (Mógłbyś dać logo do footera i też powinno wyglądać tak samo, a nie byłoby częścią header)
Tak samo z .main__form (ten form mógłby być poza main, np. w footerze i to nie powinno wpłynąc na jego styl).
BEM to zawsze BLOK__ELEMENT--MODYFIKATOR, dlatego nigdy nie może być dwóch ELEMENT (np. header__logo__inner), jeśli już bardzo nie masz pomysłu na nazwę to wtedy łącz kolejne wyrazy używając -. Czyli header__logo-inner. Dzięki temu jest BLOK - header i ELEMENT - logo-inner. (już nie wspominając, że logo nie powinien być powiązany z header)
Modyfikator (u ciebie form--email) nigdy nie może być użyty bez klasy którą się modyfikuje. Czyli jeśli piszesz form--email, to według BEM jest to edycja form, a nie jego "dziecko".
LABEL nie może być pusty, ani nie może go nie być. Możemy dać mu content i go ukryć poprzez visibility: hidden

Moja propozycja (zawartość body)

<main class="page-content"> <!-- tutaj twój .container -->
    <header>
        <figure class="coming-soon">
            <img src="assets/coming-soon-ribbon.png" alt="Coming Soon">
        </figure>
        <figure class="logo">
            <img src="assets/undraw_developer_activity_bv83.svg" alt>
        </figure>
        <h1 class="page-title">Code Academy</h1>
    </header>
    <section class="contact-section">
        <h2 class="contact-section__title">Leave your email here</h2>
        <form action="#" class="contact-section__form form form--inline"> <!-- .contact-section__form dla stylowania zależnego od miejsca, czyli np margin. .form po prostu dla ostylowania formularza, .form--inline modyfikator sprawiający, że pola formularza są obok siebie -->
            <div class="form__group">
                <label for="mail">Your email</label>
                <input type="email" name="mail">
            </div>
            <div class="form__group">
                <button type="submit" class="btn">Get early access</button>
            </div>
        </form>
    </section>
    <footer class="footer">
        <p class="footer__text">MK&copy; 2019.</p>
        <p class="footer__text">Made with <img src="assets/love.png" alt="love"> in Poland</p>
        <p class="footer__text">We are using cookies to improve your experience.</p>
    </footer>
</main>

IamMK · 2019-01-19T22:05:59Z

@mateuszszmytko poprawione

IamMK added 16 commits January 14, 2019 20:42

Responsive layout without hovers

343d3c5

Hover effect

571a43c

Focus effect

bca7801

One, small change

e706bcc

Simple backend

193ec4f

Safari position bug fixed

4910b1d

Adnotation

344bf8d

Mail validing

43c530e

Form flexing

79f741d

Session bug fixed

4af1311

Paragraph margin changed in footer

48cd2a8

Readme changed

34e3f62

Readme fusion

72853bd

Used :valid pseudoclass

61a6e3d

Email input focusing bug fixed/INvalid now

124513f

PHP in index file deleted

2e80932

D0man reviewed Jan 17, 2019

View reviewed changes

IamMK added 8 commits January 17, 2019 21:00

Name of index changed to index.html

3864747

Added pattern to email input

8ff393f

New md files structure

b444e94

Coming soon tip done

f610a62

Indents fixed and section tag deleted

a76809f

Names of classes changed

3e739b2

Migration font downloading to index file

781308c

Styling by using tag names fixed

d9258d3

IamMK added 4 commits January 19, 2019 22:47

Label displaying changed

f5c1830

Uppercase in header__title

6aa4c9f

Class names changed

5dfa7ba

Class names changed

42af6ac

Class names changed

e22f705

IamMK added 2 commits January 19, 2019 23:28

Add id for submit button

8d8e3fa

Not unique id fixed

0d3c59e

		@@ -0,0 +1,140 @@
		@import url('https://fonts.googleapis.com/css?family=Lato:300');

Comments

Conversation

IamMK commented Jan 16, 2019

Uh oh!

D0man left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

mateuszszmytko commented Jan 19, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

IamMK commented Jan 19, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mateuszszmytko commented Jan 19, 2019 •

edited

Loading