Skip to content

feat: format cds.Date, cds.Time, cds.DateTime and cds.Timestamp properties #125

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

Open
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

schiwekM
Copy link
Contributor

@schiwekM schiwekM commented Oct 7, 2024

Hi colleagues,

currently if a cds.Date property is modified it is shown as '2022-12-01' in the change log. This is not really user friendly and thus this PR adds date formatting so the dates show up as they would in UI5, e.g. '01 Dec 2022'

BR,
Marten

@schiwekM schiwekM requested a review from nkaputnik October 7, 2024 06:19
@schiwekM
Copy link
Contributor Author

@Sv7enNowitzki could this one be considered too? Would be great to have it for improved UX, because business users at not to used to the ISO format for dates.

@nkaputnik
Copy link
Contributor

Hello Marten, I have to reject this PR.
Change log Dates are indeed meant to be technical, and require to be e.g. sortable and searchable.
The use cases we encounter frequently are, that the change logs aren't even shown with the built-in UI facet, but rather in a separate, dedicated change-log UI. Since the format is statically determined at write-time, and not dynamically when reading, localizing what was written is not a good idea, especially when searching for a specific date in the logs by users in different locales.

@schiwekM
Copy link
Contributor Author

Hi Nik, I just checked again and the logic is only run in an after READ handler on ChangeView so on DB it is as intended still in ISO format.

@danjoa
Copy link
Contributor

danjoa commented Apr 29, 2025

Usually such formatting is applied on the UI, not on the API / the protocol.
I assume the problem here is that these values are intermixed with other non-date values?

@schiwekM
Copy link
Contributor Author

Usually such formatting is applied on the UI, not on the API / the protocol. I assume the problem here is that these values are intermixed with other non-date values?

Yes. The problem is that in the metadata the field is marked as a string field and thus taken by FE as is. It could also be solved if CAP would send valueChangedTo@type and valueChangedFrom@type in the result set with the EDM type to apply and UI5/FE would interpret this dynamic type accordingly for formatting.

@nkaputnik
Copy link
Contributor

Hello @schiwekM
that second proposal from you seems much more sensible to me. I'm hesitant to do any string manipulation in the backend, an annotation to instruct the front-end is much better. Any chance that you could do this yourself and submit in this PR?

Best,
Nick

@danjoa
Copy link
Contributor

danjoa commented Apr 29, 2025

Agreed, that sounds reasonable

Copy link
Collaborator

@Sv7enNowitzki Sv7enNowitzki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The flow for Node 18 needs to be deleted because CDS no longer supports Node 18. Replace 18 with 22.

@Sv7enNowitzki
Copy link
Collaborator

Based on this PR, I've enhanced some content according to the formats of Fiori elements for the languages supported by the plugin. If a language is outside the scope of these main supported languages, it will still have basic localization, but there won't be any additional processing at the moment.
https://github.com/cap-js/change-tracking/tree/main/_i18n

@Sv7enNowitzki
Copy link
Collaborator

Enhance support for cds.Time, cds.DateTime, and cds.Timestamp.

@Sv7enNowitzki Sv7enNowitzki changed the title Format date properties feat: format cds.Date, cds.Time, cds.DateTime and cds.Timestamp properties Jul 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants