Skip to content
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

Enhancement Suggestion for Time Conversion Script: Improved Accuracy and Optimization #258

Open
HelloLoser opened this issue May 27, 2024 · 3 comments
Assignees
Labels
Milestone

Comments

@HelloLoser
Copy link

Description:

I've reviewed the JavaScript snippet responsible for converting UTC times within elements bearing the .time class into the local browser time. The existing logic is commendable but presents an opportunity for refinement to enhance both accuracy and performance. Below are my observations and recommendations:

Identified Issue:

  • Inconsistent Timezone Handling: Currently, the timezone extraction relies on parsing the output of Date.toString(), which can yield variable and potentially unreliable timezone representations across different browsers and locales. This approach, using regex to match timezone names, is brittle and may not cover all international timezones comprehensively.

Proposed Enhancements:

  1. Leverage Intl.DateTimeFormat for Precise Timezone Management: To tackle the timezone inconsistency issue, I suggest employing the Intl.DateTimeFormat API. This built-in JavaScript object provides a robust mechanism for formatting dates and times according to the user's locale preferences, inherently handling timezone conversions accurately. By utilizing this API, we can avoid manual timezone string manipulations and ensure compatibility across diverse environments.

Here's the revised code snippet reflecting these improvements:

var convertTime = function() {
    $('.time').each(function() {
        var content = $(this).text();
        if(content === 'never' || content === 'unknown' || content === '') { return; }
        
        // Directly parse the UTC time string
        var utcTime = content.split(' ');
        var date = new Date(utcTime[0] + 'T' + utcTime[1] + 'Z');
        
        // Utilize Intl.DateTimeFormat for localization and timezone formatting
        var options = {
            year: 'numeric', month: '2-digit', day: '2-digit',
            hour: '2-digit', minute: '2-digit', second: '2-digit',
            hour12: false, // Use 24-hour format
            timeZone: Intl.DateTimeFormat().resolvedOptions().timeZone // Adopt the user's browser timezone
        };
        var formattedTime = new Intl.DateTimeFormat('default', options).format(date);
        
        $(this).text(formattedTime);
    });
};

By adopting this updated approach, we can significantly improve the reliability of timezone conversions while simultaneously streamlining the codebase. This change is particularly crucial for applications that demand high accuracy in timezone handling and strive for a seamless user experience across global audiences.

Please consider integrating these enhancements into the project to bolster the script's effectiveness and maintainability.

Looking forward to your feedback and further discussion on implementing these improvements.

@robertcheramy robertcheramy self-assigned this Jun 7, 2024
@robertcheramy
Copy link
Collaborator

Thank you for the issue and contribution.
I'll have a look at this.
converTime throws an error in /node/version/diffs , I want to address this too.

@robertcheramy robertcheramy added this to the 0.14.0 milestone Jun 7, 2024
@robertcheramy robertcheramy modified the milestones: 0.14.0, 0.15.0 Jun 28, 2024
@robertcheramy
Copy link
Collaborator

The whole date thing needs to be updated. This needs a change in the Oxidized-API => Changing milestone to 0.15.0

Needed changes in oxidized: transmit the date as an object, not as a string. I'm not sure yet if I want a second attribute (hash[:date_object] for backward compatibility). I probably want a brand new API for external plugins, an no deep access to internal objects of oxidized.
https://github.com/ytti/oxidized/blob/9dbc83038d764dd9f828a4c60d21eedacf6d4fec/lib/oxidized/output/git.rb#L88
https://github.com/ytti/oxidized/blob/9dbc83038d764dd9f828a4c60d21eedacf6d4fec/lib/oxidized/output/gitcrypt.rb#L116

Copy link

This issue is stale because it has been open 90 days with no activity.

@github-actions github-actions bot added the Stale label Sep 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants