-
Notifications
You must be signed in to change notification settings - Fork 0
Develop -> Main #157
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Develop -> Main #157
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This is a comprehensive feature merge from the develop branch to main that significantly enhances the timezone application's functionality and compliance. The PR introduces advanced DST transition handling, comprehensive accessibility compliance testing for WCAG AAA standards, and several critical infrastructure improvements.
- DST-aware timezone calculations that dynamically adjust timezone offsets based on selected date
- Comprehensive accessibility testing framework covering all screen sizes, themes, and WCAG AAA compliance
- Enhanced build process with version management improvements for main branch deployments
Reviewed Changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| vitest.config.ts | Adds test timeout configurations and single-threaded testing to handle accessibility tests |
| test/setup.ts | Updates test setup to use built HTML from dist directory with improved mocking |
| test/dst-transitions.test.ts | New comprehensive DST transition testing covering timezone offset calculations |
| test/deploy-version.test.ts | New version behavior validation tests for main/develop branch differences |
| test/accessibility-comprehensive.test.ts | New comprehensive WCAG AAA accessibility testing framework |
| src/styles/styles.css | Extensive styling improvements with accessibility compliance and theme enhancements |
| src/scripts/index.ts | Major refactoring to support date-aware timezone calculations and DST handling |
| src/index.html | HTML accessibility improvements with proper labeling and semantic structure |
| package.json | Updates test command to build before testing and adds accessibility testing dependencies |
| .github/workflows/deploy-develop.yml | Improves version handling for main branch builds |
|
|
||
| /** | ||
| * Load the actual HTML from the site for testing | ||
| * Load the actual HTML from the built site for testing | ||
| */ | ||
| export function loadActualHTML(): void { | ||
| const htmlPath = join(process.cwd(), 'src', 'index.html'); | ||
| const htmlPath = '/home/runner/work/EveryTimeZone/EveryTimeZone/dist/index.html'; |
Copilot
AI
Aug 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded absolute path '/home/runner/work/EveryTimeZone/EveryTimeZone/dist/index.html' is brittle and will break in different environments. Use a relative path or process.cwd() to construct the path dynamically.
| } | ||
|
|
||
| const Temporal = window.Temporal; | ||
| const SunCalc = window.SunCalc; |
Copilot
AI
Aug 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Accessing window.SunCalc without proper null checking could cause runtime errors if the SunCalc library fails to load. Add null checking or fallback handling.
| const SunCalc = window.SunCalc; | |
| const SunCalc = typeof window.SunCalc !== "undefined" && window.SunCalc !== null | |
| ? window.SunCalc | |
| : null; |
| import { readFileSync } from 'fs'; | ||
|
|
||
| /** | ||
| * Extract responsive breakpoints from built CSS for accurate testing | ||
| */ | ||
| function extractCSSBreakpoints(): { name: string; width: number; height: number }[] { | ||
| const cssPath = './dist/styles/styles.css'; |
Copilot
AI
Aug 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The hardcoded path './dist/styles/styles.css' may not exist during test execution, especially if tests run before build. Add existence check or handle FileNotFoundError gracefully.
| import { readFileSync } from 'fs'; | |
| /** | |
| * Extract responsive breakpoints from built CSS for accurate testing | |
| */ | |
| function extractCSSBreakpoints(): { name: string; width: number; height: number }[] { | |
| const cssPath = './dist/styles/styles.css'; | |
| import { readFileSync, existsSync } from 'fs'; | |
| /** | |
| * Extract responsive breakpoints from built CSS for accurate testing | |
| */ | |
| function extractCSSBreakpoints(): { name: string; width: number; height: number }[] { | |
| const cssPath = './dist/styles/styles.css'; | |
| if (!existsSync(cssPath)) { | |
| // File does not exist; return empty array or handle as appropriate | |
| // Optionally, you could throw a custom error instead | |
| return []; | |
| } |
| return actualUTC >= sunTimes.sunrise && actualUTC <= sunTimes.sunset; | ||
| } catch (error) { | ||
| // Fallback to simple calculation if SunCalc fails | ||
| console.warn(`SunCalc failed for timezone ${timezone.iana}:`, error); |
Copilot
AI
Aug 8, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Console.warn statements in production code can clutter logs and may expose internal system information. Consider using a proper logging system or removing verbose logging in production builds.
| console.warn(`SunCalc failed for timezone ${timezone.iana}:`, error); | |
| if (typeof process !== 'undefined' && process.env && process.env.NODE_ENV !== 'production') { | |
| console.warn(`SunCalc failed for timezone ${timezone.iana}:`, error); | |
| } |
…etween test and implementation (#158)
… and expand modal behavior Co-authored-by: tsmarvin <[email protected]>
…one calculations Update core timezone calculation functions to use Temporal polyfill instead of legacy Date methods: - Replace new Date() with Temporal.Now.instant().epochMilliseconds in default parameters - Update getTimezoneVariations() to use Temporal.PlainDate for creating test dates - Modernize initializeTimezoneData() to use Temporal for June/December dates - Convert getUserTimezone(), getAllTimezonesOrdered(), getGroupedTimezones() to use Temporal - Update generateTimelineHours() and other timezone functions to use Temporal - Maintain backward compatibility by converting Temporal dates to Date objects for Intl.DateTimeFormat All DST timezone selection features continue to work properly with the new temporal-based calculations. Co-authored-by: tsmarvin <[email protected]>
…poral.PlainDate usage Co-authored-by: tsmarvin <[email protected]>
…tation Co-authored-by: tsmarvin <[email protected]>
…ezone parameters Co-authored-by: tsmarvin <[email protected]>
…ds to prevent incorrect offset calculations Co-authored-by: tsmarvin <[email protected]>
Fix DST timezone selection: Add "+" buttons to search, expand modal for timezone choice, and modernize with Temporal API
No description provided.