-
Notifications
You must be signed in to change notification settings - Fork 5.5k
fix: chart history date label when older than current year #38853
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
fix: chart history date label when older than current year #38853
Conversation
|
Cursor Agent can help with this pull request. Just |
|
CLA Signature Action: All authors have signed the CLA. You may need to manually re-run the blocking PR check if it doesn't pass in a few minutes. |
3e9441a to
48c475c
Compare
48c475c to
46c212f
Compare
|
@cursor can you resolve this comment please? I agree that to be safe we should use jest fake timers. |
|
Done! I've updated the tests to use Jest fake timers to mock the system time, ensuring the tests won't become flaky after 2025. Changes made:
Updated test structure:describe('getDynamicShortDate', () => {
const originalNavigatorLanguage = navigator.language;
// Use a fixed "current year" for testing - mock system time to ensure tests don't become flaky
const MOCKED_CURRENT_DATE = new Date(2025, 6, 1, 12, 0, 0); // July 1, 2025
beforeAll(() => {
jest.useFakeTimers();
jest.setSystemTime(MOCKED_CURRENT_DATE);
Object.defineProperty(navigator, 'language', {
value: 'en-US',
configurable: true,
});
});
afterAll(() => {
jest.useRealTimers();
Object.defineProperty(navigator, 'language', {
value: originalNavigatorLanguage,
configurable: true,
});
});
// ... test cases
});All 15 tests pass and TypeScript checks pass. The changes have been pushed to the branch. Learn more about Cursor Agents |
Co-authored-by: prithpal.sooriya <[email protected]>
46c212f to
8b363c9
Compare
Builds ready [8b363c9]
UI Startup Metrics (1290 ± 101 ms)
📊 Page Load Benchmark ResultsCurrent Commit: 📄 Localhost MetaMask Test DappSamples: 100 Summary
📈 Detailed Results
Bundle size diffs [🚨 Warning! Bundle size has increased!]
|
Description
Dynamically format asset chart dates to omit the year for current year dates and include it for past/future years to improve readability.
Changelog
CHANGELOG entry: fix: chart history date label when older than current year
Related issues
Fixes: #38736 https://consensyssoftware.atlassian.net/browse/ASSETS-2116
Manual testing steps
See issue comment
Screenshots/Recordings
Before
After
https://www.loom.com/share/83ad9a83ccc9422cb24086452034ba6f
Pre-merge author checklist
Pre-merge reviewer checklist
Note
Use a dynamic short date for chart labels (time for current year, year for other years) and add unit tests.
ui/pages/asset/components/chart/asset-chart-price.tsxto usegetDynamicShortDate(date)for the hovered date label.getDynamicShortDate(date)inui/pages/asset/util.tsto switch betweengetShortDateFormatterandgetShortDateFormatterV2based on year.ui/pages/asset/util.test.tscovering multiple current/past/future year and timestamp scenarios forgetDynamicShortDate.Written by Cursor Bugbot for commit 8b363c9. This will update automatically on new commits. Configure here.