Skip to content

Develop#2722

Open
ArkadiuszBalakier wants to merge 63 commits intomate-academy:masterfrom
ArkadiuszBalakier:develop
Open

Develop#2722
ArkadiuszBalakier wants to merge 63 commits intomate-academy:masterfrom
ArkadiuszBalakier:develop

Conversation

@ArkadiuszBalakier
Copy link
Copy Markdown

- Updated package-lock.json to modify peer dependencies.
- Deleted unused font file: src/fonts/Roboto-Regular-webfont.woff.
- Removed unused SVG images: src/images/cross.svg, src/images/crown.svg, src/images/favicon.png, src/images/logo.png, src/images/menu.svg, src/images/menu_hover.svg, src/images/phone.svg.
- Deleted unused photos: src/images/photos/1.jpg, src/images/photos/2.jpg, src/images/photos/3.jpg, src/images/photos/4.jpg, src/images/photos/5.jpg, src/images/photos/6.jpg.
- Added new images: src/images/photos/Image logic.jpg, src/images/photos/Image.jpg, src/images/photos/brand-name.png.
- Removed font-face declaration from _fonts.scss.
- Updated typography styles in _typography.scss to use new extends.
- Created new title styles in blocks/title.scss.
- Refactored main.scss to use new SCSS module imports.
- Modified extends in utils/_extends.scss to use Manrope font.
- Added reset styles in utils/_reset.scss for consistent styling across browsers.
Copy link
Copy Markdown

@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.

demo link doesn’t work. deploy project using npm run deploy command

@ArkadiuszBalakier
Copy link
Copy Markdown
Author

sorry, project deployed. I didn't noticed that this task is reviewed not by AI :)

Copy link
Copy Markdown

@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.

  • burger menu icon doesn't appear
image
  • close icon touches the screen edge instead of being spaced with padding
image
  • disable page scrolling under the menu and ensure that no elements are on the same level as menu as they will be visible during section navigation
image

CHECKLIST:

  • Add a favicon
  • All Logos on the page should be links to home page
  • Change text color on hover for phone, email and address
  • When clicking on any location / address - prevent errors and make it to open location in Google Maps
  • Pictures in Gallery should increase on hover
  • Page shouldn't be reloaded on form submit
  • disable page scrolling under the menu

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

use full image with better quality
image

Copy link
Copy Markdown

@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.

  • scrollbar is still visible
image

CHECKLIST:

  • Add a favicon
  • All Logos on the page should be links to home page
  • Change text color on hover for phone, email and address
  • When clicking on any location / address - prevent errors and make it to open location in Google Maps
  • Pictures in Gallery should increase on hover
  • Page shouldn't be reloaded on form submit
  • disable page scrolling under the menu

Copy link
Copy Markdown

@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.

  • you still need to add favicon and prevent form from reloading the page on submit
  • between tablet and desktop versions images from recommended section scale up so much that their quality becomes poor. also you need to add space between images and their titles
image
  • on tablet and desktop versions paragraph has extra padding causing misalignment. also this section and contact section have side padding of 76px instead of the intended 120px
image

Copy link
Copy Markdown

@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.

Great work! just a few final fixes:

  • add success message and/or clear inputs to indicate that form was submitted successfully
  • section title should have the same max width as the rest of section
image

Copy link
Copy Markdown

@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.

“Message sent successfully!” appears even when all form inputs are empty so there's no validation for form fields

@ArkadiuszBalakier
Copy link
Copy Markdown
Author

“Message sent successfully!” appears even when all form inputs are empty so there's no validation for form fields

can we use JS ?

Copy link
Copy Markdown

@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.

tak możesz używać js a nawet już to robisz wykorzystując event.preventDefault() ;) po prostu zamiast umieszczać logikę bezpośrednio w atrybucie onsubmit to wykorzystaj już podlinkowany w html plik main.js
zobacz sobie przykład użycia: https://developer.mozilla.org/en-US/docs/Web/API/Event/preventDefault
oczywiście musisz to dostosować do swojego przypadku. najpierw używasz np. querySelectora do wybrania formularza a następnie dodajesz do niego addEventListener dla zdarzenia submit. To właśnie tam w funkcji używasz event.preventDefault() i np. reset() aby było widać że submit się udał

Copy link
Copy Markdown

@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.

na deployu z jakiegoś powodu nie ładuje ci się plik js bo jest zła ścieżka ale masz ją ustawioną poprawnie więc spróbuj wykonać ponownie deploy

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.

2 participants