Changed date strings to match request in task T2058. Each instance of toLocaleDate() has been updated to include an options parameter which presents the date in the MMM-dd-yyyy format.
Details
Details
- Reviewers
bytesofman - Group Reviewers
Restricted Project - Commits
- rABCf8759e43b370: [Cashtab] Change date format
cd web / cashtab && npm start
In the wallet tab
Ensure Tx dates are formatted correctly.
Navigate to the eTokens tab
Select an eToken
Ensure genesis date is formatted correctly.
In browser settings, change locale and run through test plan again.
Diff Detail
Diff Detail
- Repository
- rABC Bitcoin ABC
- Branch
- update-date-strings
- Lint
Lint Warnings Severity Location Code Message Warning web/cashtab/src/utils/validation.js:178 SPELL1 Possible Spelling Mistake - Unit
No Test Coverage - Build Status
Buildable 17658 Build 35142: Build Diff cashtab-tests Build 35141: arc lint + arc unit
Event Timeline
Comment Actions
Can you wrap this inside a formatDate() function in validation.js (see formatFiatBalance() which formats the currency string based on locale input params) which will then allow you to add unit tests in validation.test.js to validate its behavior when passing in null, undefined, blank strings, invalid date formats..etc? This will allow other components to confidently re-use this battle tested function later on.
| web/cashtab/src/components/Send/SendToken.js | ||
|---|---|---|
| 386 ↗ | (On Diff #31332) | Keep options here as an object instead of a variable, confusing to define it so far from where it is used, and no need to use the variable since it is only used once. |
| web/cashtab/src/components/Wallet/Tx.js | ||
| 176 ↗ | (On Diff #31332) | Call this dateFormatOptions to improve code readability |
Comment Actions
Created function which checks object for either timestampUnix or blocktime, and formats the date based on locale + options. I'd like to simplify it a bit but I am not exactly sure how to proceed. Once the function is finalized I will add unit testing.
Also, planning to change the param 'tokenObject' name because it is not descriptive.
Comment Actions
Instead of trying to handle all of the date related code, simplified the function to just accept a date string per feedback from bytesofman.
| web/cashtab/src/utils/validation.js | ||
|---|---|---|
| 180 ↗ | (On Diff #31450) | Because you are using return here, you don't need to use an if...else block if (dateString) {
return new Date(dateString * 1000).toLocaleDateString(
userLocale,
options,
);
} else {
return new Date().toLocaleDateString(userLocale, options);
}is the same as if (dateString) {
return new Date(dateString * 1000).toLocaleDateString(
userLocale,
options,
);
}
return new Date().toLocaleDateString(userLocale, options);Use the latter |
| web/cashtab/src/utils/validation.js | ||
|---|---|---|
| 217 ↗ | (On Diff #31593) | Move the definition of userLocale into the try block. Could be edge case where the navigator object is unavailable. |
Comment Actions
Please update the test plan to include testing with different locales.
I'm running into an issue like this:
- Load page in Google Chrome with en-US locale and then right click --> translate page, say to Indonesian
- Note that dates on Wallet homescreen of tx history update
- Click Token screen
- Click any other screen
- App crashes
Please debug and update test plan to include testing in multiple locales
| web/cashtab/src/utils/__tests__/validation.test.js | ||
|---|---|---|
| 232 ↗ | (On Diff #31605) | Expected test answers should be resolved, e.g. here it should be "Dec 16, 2021" |