Skip to content

add task solution#379

Open
bohdanbilozor wants to merge 7 commits into
mate-academy:masterfrom
bohdanbilozor:develop
Open

add task solution#379
bohdanbilozor wants to merge 7 commits into
mate-academy:masterfrom
bohdanbilozor:develop

Conversation

@bohdanbilozor

Copy link
Copy Markdown

@TarasHoliuk

Copy link
Copy Markdown
  1. The form should reset on the submit button click, all fields should be required. See points 13, 15 from the checklist - it's from the previous landing page task. Also, there shouldn't be a white background:

image

Mockup:
image

  1. Add cursor: pointer to buttons. Also, make them anchor links: To contact us -> to form, Shop -> to shop.

image

  1. Add hover effect to these images:
    image

  2. Open social media (all external) links in the new tab:
    image

  3. There shouldn't be padding on the mobile version:

image

Mockup:
image

@TarasHoliuk TarasHoliuk left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

GJ 👍

Let's improve it

@IvanFesenko IvanFesenko left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

@anastasiiavorobiova anastasiiavorobiova left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work! Consider fixing the previous comments
Screenshot 2024-04-02 at 14 08 21
Screenshot 2024-04-02 at 14 09 45
Consider fixing the hover style here
Screenshot 2024-04-02 at 14 10 23
Check the layout in the landscape mode

@ghost ghost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

  1. Your form should not show any errors on the form submit!
image
  1. Match the form with figma desigh:
    Figma:
image Your form: image

@bohdanbilozor bohdanbilozor requested a review from a user April 2, 2024 17:01

@Viktor-Kost Viktor-Kost left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Nice!)

To improve:

  1. check form styles
Screenshot 2024-04-02 at 20 07 46
  1. add hover effects to your links
Screenshot 2024-04-02 at 20 09 15

Recommendations:

  1. change a year to a current one
Screenshot 2024-04-02 at 20 10 06
  1. check markup
Screenshot 2024-04-02 at 20 14 06 Screenshot 2024-04-02 at 20 15 59 Screenshot 2024-04-02 at 20 18 54
  1. Suggest making sure that on desktops, the content section doesn't spread across the entire screen. Instead, it should have a maximum width. If there's a background image, image shouldn't be limited (only a section content). To do this, you need to apply the right styles in your CSS.

Why it's important:

Fits well: Setting a maximum width ensures that your design looks good on desktops without the content becoming too wide.

Easy to read: Keeping the content width in check makes it easier for people to read, as really long lines of text can be hard to understand.

Same look: Having a maximum content width helps maintain a consistent design, especially when using background images that go beyond the content area.

Easy to organize: Limiting the content width makes it simpler to organize your content on the page. This makes it easier for developers and designers to manage.

@anastasiiavorobiova anastasiiavorobiova left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Great work! Almost done!
Screenshot 2024-04-03 at 14 28 13
Consider fixing the white background here

@anastasiiavorobiova anastasiiavorobiova left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Awesome work!

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.

5 participants