Develop#2743
Conversation
|
Zrobiłam przez przypadek inaczej z rozbiegu i dodałam wszystkie bloki na raz. Problem jest również z galerią. Nie działa tak jak powinna. Nie wiem jak ten problem rozwiązać. |
natalia-klonowska
left a comment
There was a problem hiding this comment.
- ensure that no elements are on the same layer as menu as they remain visible during section navigation and disable scrolling under menu
- Add a favicon
- Change text color on hover for phone, email and address
- When you click on phone icon or phone number in contacts section, make sure that there is no 404 error, make it a real link to start a call on device
- When clicking on any location / address - prevent errors and make it to open location in Google Maps
- Pictures in Gallery should increase on hover
- apply validation of the form fields (required, email / tel etc.), then it is clear in what format to enter the data
- Form shouldn't be submitted if some of the fields are not filled
- Make sure everything looks neat on mobile and without horizontal scrolling:
Zibi95
left a comment
There was a problem hiding this comment.
Hej. Bravo jeszcze trochę szlifów trzeba zrobić, ale prawie gotowe. Ogolnie sprawdz checklist.md, bo niektore rzeczy nie sa do konca zrobione. Na przyklad przy submicie formularza odswieza sie strona.
- powinno byc wycentrowane horyzontalnie
-
jak mamy otwarte menu, to mamy scroll. W checklist.md jest kawalek skryptu, ktory ukryje scroll.
-
przy szerokosci 1040 zdjecie nie pokrywa calosci sekcji hero
- sekcja na screenie nie jest wycentrowana
- layout wyglada zle, bo niektore sekcje zaczynaja sie wczesniej niz inne
natalia-klonowska
left a comment
There was a problem hiding this comment.
większość problemów z ostatniego review nie została rozwiązana:
- numer nadal nie jest poprawnie wycentrowany
- scroll nie powinien pojawiać się gdy otwarte jest menu
- od szerokości 1020px pojawia się scroll poziomy przez grid w sekcji contacts oraz psuje się layout innej sekcji
- wycentruj oraz elementy powinny dostosowywać się do zmiany szerokości przeglądarki
natalia-klonowska
left a comment
There was a problem hiding this comment.
- ikony w nawigacji nie są poprawnie wycentrowane oraz powinny mieć takie samo położenie co przy otwartym menu
- w tym miejscu masz dwa razy nałożone boczne marginesy (na main i na sekcję) ale żadne z nich ostatecznie nie wpływają na zdjęcia. odległości między tytułami a zdjęciami też są błędne i dobrze byłoby je uprościć aby nie były zależne zarówno od wartości poszczególnych elementów oraz gap ustawionego dla całej sekcji
- popraw odległości oraz font-weight
- sekcja contact us ma zbyt duże marginesy boczne
- nadal jest scroll przy otwartym menu
|
Poprawiłam mam nadzieję już wszystkie błędy. Jedyny problem jest taki, że u mnie nie ma scrolla, nie był on już wcześniej widoczny więc myślałam, że jest w porządku - dodałam kod z checklisty :) Proszę też o łaskawsze podejście do sprawdzania - w zadaniu było powiedziane że stronka nie musi być wykonana pixel perfect :) |
natalia-klonowska
left a comment
There was a problem hiding this comment.
jeszcze raz użyj komendy npm run deploy aby zaktualizować demo
Poprawiłam mam nadzieję już wszystkie błędy. Jedyny problem jest taki, że u mnie nie ma scrolla, nie był on już wcześniej widoczny więc myślałam, że jest w porządku - dodałam kod z checklisty :) Proszę też o łaskawsze podejście do sprawdzania - w zadaniu było powiedziane że stronka nie musi być wykonana pixel perfect :)
Projekt nie musi być wykonany pixel perfect ale różnice w odległościach między elementami są na tyle duże i częste że widać je od razu przy porównaniu z designem. szczególnie że problemem nie jest nawet sama wartość odstępów tylko brak spójności. marginesy i paddingi z różnych elementów nakładają się na siebie w chaotyczny sposób przez co brakuje konsekwencji w ich nadawaniu
|
zaktualizowałam demo |
natalia-klonowska
left a comment
There was a problem hiding this comment.
- przy 1020px pojawia się scroll poziomy
- nadal nawigacja nie jest poprawnie wycentrowana. ma dodatkowy padding z prawej strony oraz logo ma dodatkowy margines z lewej
- lepiej będzie wyglądać jeśli tekst pozostanie wycentrowany razem ze zdjęciem
- tekst pod zdjęciami jest przesunięty za bardzo w prawo
- pozycja ikon z nawigacji powinna być taka sama dla otwartego i zamkniętego menu aby całość wyglądała bardziej spójnie
|
Poprawiłam niektóre rzeczy, natomiast co do np nawigacji czy niektórych wcięć to są one zrobione zgodnie z projektem w trzech różnych wariantach ekranowych. Nie było napisane że layout musi się zgadzać od 320 do 1260, tylko 320, 744, 1260. W checkliście również nie jest to ujęte. W filmach instruktażowych np przy budowaniu nawigacji widać przeskok między mniejszym a większym ekranem w ikonach itd. |
natalia-klonowska
left a comment
There was a problem hiding this comment.
Poprawiłam niektóre rzeczy, natomiast co do np nawigacji czy niektórych wcięć to są one zrobione zgodnie z projektem w trzech różnych wariantach ekranowych. Nie było napisane że layout musi się zgadzać od 320 do 1260, tylko 320, 744, 1260. W checkliście również nie jest to ujęte. W filmach instruktażowych np przy budowaniu nawigacji widać przeskok między mniejszym a większym ekranem w ikonach itd.
design zawsze pokazuje konkretne breakpointy a zadaniem developera jest stworzenie responsywnej strony która będzie w spójny sposób dostosowywać się do każdego rozmiaru ekranu. na każdej wersji w figmie widać że ikony zamykania i otwierania menu są w tym samym miejscu więc tak też powinno pozostać niezależnie od szerokości okna przeglądarki
- uprość zmieniając kolumnę na której powinna się kończyć nawigacja zamiast używać translate to dodatkowego przesunięcia ikon. dodatkowo powielasz paddingi dodając je jednocześnie na header i nav
- nadal pojawia się scroll poziomy przy 1020px
- marginesy boczne powinny być spójne dla całej strony





DEMO LINK