- 
          
 - 
                Notifications
    
You must be signed in to change notification settings  - Fork 6.9k
 
Fix(i18n): refactor secondsToHumanReadableFormat #6281
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: master
Are you sure you want to change the base?
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 PR modifies the filtering logic in the RelativeTimeFormatter class to ensure integer parts from Intl.RelativeTimeFormat are always included in the formatted output, regardless of their position in the parts array. Previously, both literal and integer parts at index 0 were excluded, which could potentially skip the numeric value if it appeared first in the array.
- Modified the filter condition in 
toFormattedPartto always include integer type parts - Literal parts at index 0 are still excluded while all other positions are included
 - This ensures numeric values are never accidentally filtered out based on their array position
 
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| 
           I noticed this a few day ago too. Even after this patch,  I think we need some ideas to overcome with this for all languages.  | 
    
| 
           Actually we have this sentence:  My idea is changing to this: The second part don't have to translate.  | 
    
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
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot <[email protected]>
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
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
          
 
 Another solution is dayjs, but it require the locale plugin of each locales to be loaded, which may be somehow inconvenient.  | 
    
          
 Agree, this will make the UI clearer. However, we can still support multiple languages.  | 
    
| 
           https://caniuse.com/wf-intl-duration-format It is baseline available enough for my taste. Regarding if the wording is awkward, you two will need to judge that, I can't read Chinese ^^  | 
    
| 
           This function looks promising, it is ok to use, but surely needs try-catch or polyfill.  | 
    
📋 Overview
Currently
secondsToHumanReadableFormatis using theIntl.RelativeTimeFormatlibrary, assumingformatToPartsreturns values like['in', 60, 'seconds'], thus excluding the first 'in' in the array and returning the final result. However, when the language is Chinese,formatToPartsreturns[1, '分钟后'], in which case excluding the first value would return an incorrect value.Fix the incorrect display. See the screenshots below.
🛠️ Type of change
📄 Checklist
📷 Screenshots or Visual Changes
Before:

(It displays "分钟后 秒钟后" in Chinese, which is meaningless.)
(It works well in English.)
After:

(It displays "1分钟40秒" in Chinese.)
(The English UI looks better too.)