CYF-ITP-South-Africa | Simphiwe Mabuya | Module-Data-Groups | Sprint-3 - Alarm-Clock #276
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…uerySelector with their CSS id selectors
…meRemaining function within
6 tasks
KudahShambare
suggested changes
Jan 9, 2025
There was a problem hiding this comment.
just a minor improvement you cand add o your script tag
There was a problem hiding this comment.
- When a user starts another timer while another one is still running, your code is breaking, you need to find a way you can handle that case ( you can either overwrite the old timer with the new one, you may not allow a user to overwriter a running timer, e.t.c)
- For clarity to the user, you may specify which units are they entering in the textbox (minutes?, hours?, seconds,days?)
KudahShambare
added
Reviewed
…then added the defer attribute to my script
|
Greetings Kudah
Complements of the New Season!
I am sure that this will find you well and in great spirit this evening.
Thanks a million for your constructive feedback.
I have thus attended to your suggestions and advice re fixing my Alarm
Clock code.
On Sprint-3/alarmclock/index.html
<#276 (comment)>:
I have edited my HTML by adding the defer attribute in my script tag and
clarified to the user which units are they entering in the text box.
On Sprint-3/alarmclock/alarmclock.js
<#276 (comment)>:
I have opted to modify my code to overwrite the existing timer whenever a
new timer is set.
Thanks a lot again for your feedback..it means a lot to me.
Best regards
Simphiwe Mabuya
…On Thu, 9 Jan 2025 at 10:48, Kudakwashe Shambare ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
On Sprint-3/alarmclock/index.html
<#276 (comment)>
:
just a minor improvement you cand add o your script tag
https://www.w3schools.com/tags/att_script_defer.asp
------------------------------
On Sprint-3/alarmclock/alarmclock.js
<#276 (comment)>
:
- When a user starts another timer while another one is still running,
your code is breaking, you need to find a way you can handle that case (
you can either overwrite the old timer with the new one, you may not allow
a user to overwriter a running timer, e.t.c)
- For clarity to the user, you may specify which units are they
entering in the textbox (minutes?, hours?, seconds,days?)
—
Reply to this email directly, view it on GitHub
<#276 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BIXBQLK4TC5YTCXYT2NEX7T2JYZVPAVCNFSM6AAAAABUWO77NSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKMZZGA4DMMZQGE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Simphiwe-Mabuya
added
Complete
|
Hi Simpiwe , thank you for the compliments of the new season. Everything is looking good now. |
KudahShambare
approved these changes
Jan 13, 2025
|
Thanks a million Kudah 🙏🏾🙏🏾🙏🏾
…On Mon, 13 Jan 2025 at 23:31, Kudakwashe Shambare ***@***.***> wrote:
***@***.**** approved this pull request.
—
Reply to this email directly, view it on GitHub
<#276 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/BIXBQLNYE656TCHSPDAN2YT2KQWCHAVCNFSM6AAAAABUWO77NSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMZDKNBXHE2TQNBTHA>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Learners, PR Template
Self checklist
Changelist
Briefly explain your PR.
Questions
Ask any questions you have for your reviewer.