Skip to content
This repository was archived by the owner on Jan 14, 2024. It is now read-only.

London9-OlehPysmenko-Week1 - #240

Open
tiritaki wants to merge 7 commits into
CodeYourFuture:masterfrom
tiritaki:master
Open

London9-OlehPysmenko-Week1#240
tiritaki wants to merge 7 commits into
CodeYourFuture:masterfrom
tiritaki:master

Conversation

@tiritaki

Copy link
Copy Markdown

Volunteers: Are you marking this coursework? You can find a guide on how to mark this coursework in HOW_TO_MARK.md in the root of this repository

Your Details

  • Your Name: Oleh Pysmenko
  • Your City: London
  • Your Slack Name: tiritaki

Homework Details

  • Module:
  • Week:1

Notes

  • What did you find easy?

  • What did you find hard?

  • What do you still not understand?

  • Any other notes?

Comment thread css/style.css
/* hero */

.hero {
height: 80px;

@nuclear-pasta nuclear-pasta Nov 1, 2022

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

you are limiting the height of the div but the content of the children is too long for the height suggested. so it is actually not fitting in the space of the 80px. And you probably don't want put any restriction on the height of this

Comment thread css/style.css
}

.scooter-background {
position: relative;

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

normally you would have the whole image as vector, included with background image and shadow. What you are trying to achieve is way harder cause you are recreating the whole design from scratch but with css, and is pushing you to use a lot of relative and absolute positioning which makes more complex having a responsive design

@nuclear-pasta

Copy link
Copy Markdown

it is very good! Nice jobs. I think you made your life harder with reassembling the image yourself, but other than that it is great. The Loom I sent you last time and some comments are just for details

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants