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

Develop- first block #2681

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

Conversation

OlkowskaK
Copy link

Copy link

@danon321 danon321 left a comment

Choose a reason for hiding this comment

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

Link do demo wyrzuca 404 :c
Jak masz jakies problemy to zapraszamy na QnA albo pisz na czacie :))

@OlkowskaK OlkowskaK requested a review from danon321 February 19, 2025 07:53
Copy link

@danon321 danon321 left a comment

Choose a reason for hiding this comment

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

Jedyne co widze po wejsciu w link do demo to:
image

@OlkowskaK OlkowskaK requested a review from danon321 February 19, 2025 12:28
Copy link

@Zibi95 Zibi95 left a comment

Choose a reason for hiding this comment

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

I've reviewed the project and noticed a few issues that need attention:

  1. Header Section:

    • The background image quality is poor and needs to be replaced with a higher resolution version
    • Consider optimizing the image for web use (compression while maintaining quality)
  2. Hours Section:

    • The layout is not properly aligned
    • Elements need proper spacing and organization
    • Consider using CSS Grid or Flexbox for better structure

Recommendation for workflow:

  1. Please complete one section at a time
  2. Add comments in the code indicating which sections are ready for review

@OlkowskaK OlkowskaK requested a review from Zibi95 February 21, 2025 08:07
Copy link

@danon321 danon321 left a comment

Choose a reason for hiding this comment

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

Pierwsza sekcja po headerze na wiekszych rozdzielczosciach ci sie rozjezdza:
image

Zgodnie z figma teks w headerze powinien byc wiekszy i wyzej obrazka:
image
vs
image

Sekcja "Now On View" nie jest skonczona wiec nawet nie wiem czy sprawdzac

@OlkowskaK OlkowskaK requested a review from danon321 February 21, 2025 13:13
Copy link

@natalia-klonowska natalia-klonowska left a comment

Choose a reason for hiding this comment

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

Check and adjust the spacing between elements in this section
demo vs figma design:
image

adjust position of first and last image
change sizes of images in second row
demo vs figma design:
image

Copy link

@danon321 danon321 left a comment

Choose a reason for hiding this comment

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

image

Tutaj przy ekranie 1900px rozdzielczosci kontent rozjezdza ci sie na cała szerokosc ekranu.

image

Tu na figmie jak widzisz wszystko trzyma sie w tak zwanym kontenerze o szerokosci 1020px

Mozesz osiagnac ten efekt dodajac tam na sekcje klase .container
ktora bedzie wygladac mniej wiecej tak
{ max-width:1020px; width:100%; margin: 0 auto; }

@OlkowskaK OlkowskaK requested a review from danon321 March 4, 2025 12:16
Copy link

@natalia-klonowska natalia-klonowska left a comment

Choose a reason for hiding this comment

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

  • in More to Explore section all paragraphs don't spread out across the same number of lines as in the design:
    image
  • first image is still a little off. it needs to be slightly zoomed in
    image

Copy link

@natalia-klonowska natalia-klonowska left a comment

Choose a reason for hiding this comment

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

  • Pictures in Gallery should increase on hover
  • 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
  • Page shouldn't be reloaded on form submit (https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault)
    -text should start with the same margin as the placeholder instead of touching the edges of the input field:
    image

Copy link

@danon321 danon321 left a comment

Choose a reason for hiding this comment

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

Super robota!!!

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.

4 participants