-
Notifications
You must be signed in to change notification settings - Fork 0
fix: date in tabella per trasparenza #112
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: main
Are you sure you want to change the base?
Conversation
|
aggiungo però che c'era anche un errore con 'HH:MM' dove invece deve essere 'HH:mm' perchè MM è il mese, mentre mm i minuti |
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 fixes date handling and timezone issues in transparency tables by introducing timezone-aware date formatting using moment-timezone, and correcting a date format typo where month (MM) was used instead of minutes (mm).
Key Changes:
- Added timezone support to DatetimeWidget using moment-timezone with timezone from server configuration
- Fixed date formatting in transparency tables to conditionally show time when present in publication dates
- Corrected format string from 'HH:MM' to 'HH:mm' to properly display minutes
Reviewed Changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/customizations/volto/components/theme/Widgets/DatetimeWidget.jsx | New customization that applies timezone-aware formatting to datetime values |
| src/components/View/Bando/Dates.jsx | Updated to use timezone-aware moment formatting instead of pre-formatted dates |
| src/components/Blocks/Listing/Table/TableTemplate.jsx | Fixed minute format typo and added logic to conditionally display time based on date content |
| package.json | Added moment-timezone dependency |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
|
||
| import { useSelector } from 'react-redux'; | ||
| import { viewDate } from 'io-sanita-theme/helpers'; | ||
| import moment from 'moment-timezone'; |
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.
Possiamo importarlo lazy? Non ho guardato come sia stato importato altrove nel repo
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.
Possiamo importarlo lazy? Non ho guardato come sia stato importato altrove nel repo
il lavoro di avere moment lazy su Volto è un lavoro "a metà", fare una libreria lazy "a metà" è più la perdita che il guadagno. Però, nello specifico qui posso faare moment-timezone lazy e vedere se c'è un guadagno reale.
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.
sposto la verifica per il lazy su una issue a parte
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Copilot <[email protected]>
razzle.config.js del progetto (nel nostro caso io-sanita)
In generale il problam è che aver messo in modo esplicito deii nomi dei campi e come formatttarli rende poco flessibile la tabella, perchè in alcuni casi si vorrebbe che dei campi con quel nome avessero sia data che ora.
Ho riportato qui dei casi spsecifiche che si potrebbero anche gestire con una customization completa del jsx, ma servirebbe a mio avviso ripensare a montet la cosa.
Lascio in draft avendo comunque risolto il caso specifico con una customization