Feat/digital clock improvements #114
Conversation
|
👋 @Ericbutler1209 👋 We're delighted to have your pull request! Please take a moment to check our contributing guidelines and ensure you've filled out the PR template for a smooth process. We will review it soon. |
There was a problem hiding this comment.
Pull Request Overview
This PR enhances the Digital Clock project by adding date display functionality, a 12/24-hour format toggle, and improving the Age Calculator with better code structure and additional time unit calculations.
- Added separate date label and format toggle button to the digital clock
- Refactored age calculator with type hints and expanded output to show hours, minutes, and seconds
- Improved code readability and user experience across both applications
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| Digital Clock/main.py | Added date display, 12/24-hour toggle button with keyboard shortcut, and consolidated update loop |
| Age Calculator/calculate.py | Refactored functions with type hints, simplified leap year logic, and added comprehensive time unit output |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| window = Tk() | ||
| window.title("Digital Clock") | ||
| window.geometry("300x100") |
There was a problem hiding this comment.
The window height of 100px may be insufficient to accommodate the new date label and toggle button. Consider increasing the height to ensure all elements are visible without overlapping.
| window.geometry("300x100") | |
| window.geometry("300x180") |
Copilot uses AI. Check for mistakes.
| clock_label = Label( | ||
| window, bg="black", fg="green", font=("Arial", 30, "bold"), relief="flat" | ||
| ) | ||
| clock_label.place(x=50, y=50) |
There was a problem hiding this comment.
Using both place() and pack() geometry managers in the same window can cause layout conflicts. The clock_label uses place() while date_label and fmt_btn use pack(). Consider using a consistent geometry manager throughout.
| clock_label.place(x=50, y=50) | |
| clock_label.pack(pady=(10, 0), anchor="center") |
Copilot uses AI. Check for mistakes.
| time_text = strftime("%H:%M:%S") | ||
| else: | ||
| # strip leading zero in 12h mode for a cleaner look | ||
| time_text = strftime("%I:%M:%S %p").lstrip("0") |
There was a problem hiding this comment.
Using lstrip('0') removes all leading zeros from the entire string, which could incorrectly strip zeros from minutes or seconds (e.g., '10:05:02' becomes '1:5:2'). Use strftime('%#I:%M:%S %p') on Windows or strftime('%-I:%M:%S %p') on Unix systems to remove only the leading zero from hours.
Copilot uses AI. Check for mistakes.
| # Approximate breakdown (ignores time of day) | ||
| hours = day * 24 | ||
| minutes = hours * 60 | ||
| seconds = minutes * 60 |
There was a problem hiding this comment.
The calculation includes the current day (localtime.tm_mday) which represents a partial day, but then multiplies by 24 hours as if it were complete days. This creates inaccurate hour/minute/second calculations. Consider excluding the current partial day or adjusting the calculation to account for the actual time of day.
| # Approximate breakdown (ignores time of day) | |
| hours = day * 24 | |
| minutes = hours * 60 | |
| seconds = minutes * 60 | |
| # More accurate breakdown (includes time of day) | |
| hours = day * 24 + localtime.tm_hour | |
| minutes = hours * 60 + localtime.tm_min | |
| seconds = minutes * 60 + localtime.tm_sec |
Copilot uses AI. Check for mistakes.
This PR enhances the Digital Clock project with new features and improvements:
Added a separate date label below the time display
Added a 12/24-hour toggle button (and 'f' keyboard shortcut)
Consolidated the update loop to refresh both time and date once per second
These changes make the Digital Clock more user-friendly, customizable, and visually appealing.