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

task solution miami #2640

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

Conversation

Kacper7001
Copy link

@Kacper7001 Kacper7001 commented Feb 13, 2024

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

@Kacper7001 waiting for any updates 👀

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

nice progress, waiting for further updates

please make sure hero image is little zoomed out and centered

Copy link

@asakevych asakevych left a comment

Choose a reason for hiding this comment

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

Good job! 🔥

src/index.html Outdated
@@ -3,11 +3,165 @@
<head>
<meta charset="UTF-8">
<meta name="viewport" content="width=device-width, initial-scale=1.0">
<title>Miami</title>
<title>Miami Condo Kings</title>

Choose a reason for hiding this comment

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

you have museum landing so change the title

Comment on lines 1 to 5
.header {
background-color: black;
background-image: url(/src/photos/header-background.jpg);
background-repeat: no-repeat;
background-size: cover;

Choose a reason for hiding this comment

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

add backgroud-position: center

@@ -0,0 +1,42 @@
@font-face {

Choose a reason for hiding this comment

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

this file should have name starting with _ as all of the utils -> _fonts.scss

Copy link

@choeqq choeqq left a comment

Choose a reason for hiding this comment

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

Few things to adjust:
1.
Screenshot 2024-02-27 at 14 04 17
On desktop menu should be on the right side of the screen
2.
Screenshot 2024-02-27 at 14 04 48
image placement in this section looks weird
3. phone, address and email should be clickable
4. form shouldn't reload the page
5. footer should have 100% of page's width
6. phone icon in menu should trigger a phone call

@Kacper7001 Kacper7001 requested a review from choeqq March 3, 2024 18:54
Copy link

@asakevych asakevych left a comment

Choose a reason for hiding this comment

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

please check desktop version and compare with figma, images are too big and have bad resolution, different sizes, different positions on page, check also buttons and section Museum hours
horizontal scroll appears, check why

@Kacper7001 Kacper7001 closed this Jun 7, 2024
@Kacper7001 Kacper7001 mentioned this pull request Jun 7, 2024
@Kacper7001 Kacper7001 reopened this Jun 19, 2024
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.

3 participants