Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ This is possible because [we use the Parcel library](https://en.parceljs.org/scs
## Checklist

❗️ Replace `<your_account>` with your GitHub username and copy the links to the `Pull Request` description:
- [DEMO LINK](https://<your_account>.github.io/layout_stop-watch/)
- [DEMO LINK](https://karolina0372.github.io/layout_stop-watch/)

❗️ Copy this `Checklist` to the `Pull Request` description after links, and put `- [x]` before each point after you checked it.

Expand Down
5 changes: 4 additions & 1 deletion src/index.html
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,9 @@
/>
</head>
<body>
<h1>Stop watch</h1>
<div class="stopwatch stopwatch--speed-up">
<div class="stopwatch__minutes"></div>

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

According to the description, "The starting position of the hands should be at the top." With the current SCSS (using transform-origin: left center and rotation around a horizontal axis from the center), these hands will appear horizontal, not pointing to the top of the circle. You should adjust the hand positioning/transform logic so that, with this markup, the hands are visually pointing up at the start of the animation.

<div class="stopwatch__seconds"></div>
</div>
</body>
</html>
73 changes: 73 additions & 0 deletions src/styles/index.scss
Original file line number Diff line number Diff line change
@@ -1,3 +1,76 @@
@mixin hand($width, $height, $bg-color) {
position: absolute;
left: 50%;
top: 50%;
width: $width;
height: $height;
background-color: $bg-color;
}

body {
margin: 0;
display: flex;
flex-direction: column;
align-items: center;
justify-content: center;
height: 100vh;
}

.stopwatch {
position: relative;
width: 80vmin;
height: 80vmin;
border: 1vmin dotted #000;
border-radius: 50%;

&__seconds {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The task explicitly requires: "The starting position of the hands should be at the top." With the current combination of transform-origin: left center on the elements and the rotate(-90deg) / rotate(270deg) in the keyframes, the hands are horizontally aligned (pointing to the right) at the start, not vertically toward the top. This violates the core requirement and the previous review note about fixing the initial position of both hands.

@include hand(38vmin, 1.5vmin, #2c8000);

transform-origin: 0 50%;
transform: translateY(-50%) rotate(-90deg);
animation: rotate 60s linear infinite;
Comment on lines +26 to +31

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This still violates the requirement "The starting position of the hands should be at the top" and the previous review’s correction: with left: 50%, top: 50%, transform-origin: 0 50%, and the current keyframes, the hands initially extend horizontally from the center instead of pointing to 12 o’clock. You need to adjust the hands’ geometry and transform-origin so the base is at the center of the stopwatch and the rectangle points straight up at the initial frame while rotating around that center.

}

&__minutes {
@include hand(20vmin, 3vmin, #0700ff);

transform-origin: 0 50%;
transform: translateY(-50%) rotate(-90deg);
animation: rotate 3600s steps(60) infinite;
Comment on lines +34 to +39

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This shares the same issue as the seconds hand: the minutes hand’s initial transform and transform-origin make it start horizontally rather than at the top, which conflicts with the requirement "The starting position of the hands should be at the top." Consider using a pattern like transform-origin: center bottom and animating from rotate(0deg) to rotate(360deg) so it starts at 12 o’clock and rotates correctly.

}

&::before {
content: '';
position: absolute;
left: 50%;
top: 50%;
width: 5vmin;
height: 5vmin;
background-color: #f6a603;
border-radius: 50%;
transform: translate(-50%, -50%);
z-index: 1;
}

&--speed-up {
.stopwatch__seconds {
animation-duration: 10s;
}

.stopwatch__minutes {
animation-duration: 600s;
}
}
}

@keyframes rotate {
from {
transform-origin: left center;
transform: translateY(-50%) rotate(-90deg);
}

to {
transform-origin: left center;
transform: translateY(-50%) rotate(270deg);
Comment on lines +66 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This animation uses from/to + transform with rotate, which is good, but the chosen transforms (translateY(-50%) rotate(-90deg) to translateY(-50%) rotate(270deg)) combined with transform-origin: left center keep the hands horizontal. You need to change the origin and/or transform so that at from the visible hand points straight up at the top of the stopwatch while still rotating around the center.

Comment on lines +66 to +74

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This keyframes definition violates checklist item #1: "Keyframes implemented using from/to + transform with rotate property" and the requirement that the hand starts at the top. You are animating translateY(-50%) rotate(-90deg)translateY(-50%) rotate(270deg), which makes the initial position horizontal at 9 o’clock instead of 12 o’clock. Rework the keyframes so they animate only the rotation (e.g. transform: rotate(0deg) to transform: rotate(360deg)) with a fixed transform-origin that keeps the base in the center and the hand pointing up at the start.

}
}
Loading