-
Notifications
You must be signed in to change notification settings - Fork 29
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
minor ux improvements for scm #1160
Conversation
return '--'; | ||
} | ||
|
||
return moment(t.seconds * 1000).format('DD-MM-YYYY HH:mm:ss A'); |
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.
For future consideration: consolidate with date formatting logic in https://github.com/cdapio/cdap-ui/blob/develop/app/cdap/services/DataFormatter/index.ts
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.
done.
f62b5e8
to
5314d23
Compare
? T.translate(`${PREFIX}.operationRanFor`).toString() | ||
: T.translate(`${PREFIX}.operationRunningFor`).toString(); | ||
|
||
return `${startedPrefix} ${startTime}, ${timeTakenPrefix} ${timeTaken}`; |
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.
We should not be using template literals to generate localized strings - some languages will expect segments to be in a different order. To be fully internationalized, this should be done through T.translate
(L102 also)
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.
Agreed. This current implementation may not work properly in all cases, especially in RTL languages. Will update it.
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.
Updated.
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.
One question about i18n - up to you if you want to address it.
5314d23
to
d8e5fcc
Compare
d8e5fcc
to
ec04bcd
Compare
[🍒] minor ux improvements for scm (from #1160)
minor ux improvements for scm
Description
Improves data fetching in the scm sync tab, and some copy changes.
Contains a subset of changes from #1157 , as the former PR got larger and not all changes there are cleared to be included in the release 6.10.0
PR Type
Links
Jira: CDAP-20917
Test Plan
e2e tests added already.
Screenshots
NA