Glasgow | 25-ITP-Sep | Abraham-Habte | Sprint 3| alarmClock app#842
Glasgow | 25-ITP-Sep | Abraham-Habte | Sprint 3| alarmClock app#842Abrsh100 wants to merge 2 commits intoCodeYourFuture:mainfrom
Conversation
Abrsh100
added
the
Needs Review
illicitonion
added
Review in progress
illicitonion
left a comment
illicitonion
left a comment
There was a problem hiding this comment.
This is generally looking good, but there's a question about exactly what input you should be taking from the user. Also, look out for places you can avoid duplication!
Sprint-3/alarmclock/index.html
Outdated
| <h1 id="timeRemaining">Time Remaining: 00:00</h1> | ||
| <label for="alarmSet">Set time to:</label> | ||
| <input id="alarmSet" type="number" /> | ||
| <input id="alarmSet" type="datetime-local" required /> |
There was a problem hiding this comment.
The requirements ask you to have an input which you can put a number of seconds into, but when I open your app, I see a date-picker. Can you update this to take a number of seconds instead?
There was a problem hiding this comment.
ok, thanks for the information, based on the requirment i change to number and minimum input to be 1 and for clarification i put placeholder to be second
There was a problem hiding this comment.
Looks good! Be aware that placeholders aren't generally accessible (see https://www.deque.com/blog/accessible-forms-the-problem-with-placeholders/) so you may want to look at other ways of conveying this information :)
illicitonion
added
Reviewed
Abrsh100
added
Needs Review
bp7968h
added
Review in progress
bp7968h
left a comment
bp7968h
left a comment
There was a problem hiding this comment.
Nice attempt, a small change otherwise looks good to me!
| <h1 id="timeRemaining">Time Remaining: 00:00</h1> | ||
| <label for="alarmSet">Set time to:</label> | ||
| <input id="alarmSet" type="number" /> | ||
| <input id="alarmSet" type="number" min= "1" placeholder="Second" required /> |
There was a problem hiding this comment.
This input field looks good for normal people, how would a screen reader identify what to input, can this be improved for screen readers?
bp7968h
added
Reviewed
| const updateCountdown = () => { | ||
| const timeLeft = endTime - Date.now(); |
There was a problem hiding this comment.
This code is still inconsistently indented compared to the rest of the file
3 participants
Self checklist
Changelist
I added code to the fucnction set alarm and modifise the input in html.
Questions
No questions