korbit-ai[bot] commented on code in PR #31019:
URL: https://github.com/apache/superset/pull/31019#discussion_r1898412485
##########
superset-frontend/src/components/ListView/Filters/DateRange.tsx:
##########
@@ -69,16 +69,16 @@ function DateRangeFilter(
<RangePicker
placeholder={[t('Start date'), t('End date')]}
showTime
- value={momentValue}
- onChange={momentRange => {
- if (!momentRange) {
+ value={dayjsValue}
+ onCalendarChange={(dayjsRange: [Dayjs, Dayjs]) => {
Review Comment:
### Incorrect DatePicker Event Handler <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The DateRangePicker's onCalendarChange handler is used instead of onChange,
which can lead to incorrect date selection behavior. onCalendarChange fires
when navigating through the calendar, while onChange fires when a date is
actually selected.
###### Why this matters
Using onCalendarChange can trigger premature submissions before the user has
finalized their date selection, leading to unintended filter updates and
potential user confusion.
###### Suggested change
Replace onCalendarChange with onChange:
onChange={(dayjsRange: [Dayjs, Dayjs] | null) => {
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:c123f3bd-a7e8-4aef-80b5-d318a9dc3336 -->
##########
superset-frontend/src/components/DatePicker/DatePicker.stories.tsx:
##########
@@ -16,22 +16,26 @@
* specific language governing permissions and limitations
* under the License.
*/
-import { DatePickerProps, RangePickerProps } from 'antd/lib/date-picker';
+import { DatePickerProps } from 'antd-v5';
+import { RangePickerProps } from 'antd-v5/es/date-picker';
import { DatePicker, RangePicker } from '.';
export default {
title: 'DatePicker',
component: DatePicker,
};
-const commonArgs = {
- allowClear: true,
+const commonArgs: DatePickerProps = {
+ allowClear: false,
autoFocus: true,
- bordered: true,
disabled: false,
+ format: 'YYYY-MM-DD hh:mm a',
Review Comment:
### Incompatible Date-Time Format <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
The date-time format 'YYYY-MM-DD hh:mm a' is a moment.js format. Dayjs
requires 'YYYY-MM-DD HH:mm A'.
###### Why this matters
Using moment.js date-time format with dayjs will result in incorrect date
and time formatting in the UI.
###### Suggested change
Change the format to 'YYYY-MM-DD HH:mm A' to match dayjs formatting
requirements.
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:de3ef243-fbea-4836-95f6-948364eecd48 -->
##########
superset-frontend/src/explore/components/controls/DateFilterControl/components/CustomFrame.tsx:
##########
@@ -160,13 +160,13 @@ export function CustomFrame(props: FrameComponentProps) {
<Row>
<DatePicker
showTime
- defaultValue={dttmToMoment(sinceDatetime)}
- onChange={(datetime: Moment) =>
- onChange('sinceDatetime', datetime.format(MOMENT_FORMAT))
+ defaultValue={dttmToDayjs(sinceDatetime)}
Review Comment:
### DatePicker Uncontrolled State Issue <sub></sub>
<details>
<summary>Tell me more</summary>
###### What is the issue?
Using defaultValue with DatePicker instead of value prop means the component
won't update when sinceDatetime changes externally
###### Why this matters
The DatePicker won't reflect updates to sinceDatetime from outside the
component, creating a disconnect between the actual state and what's displayed
###### Suggested change
Replace defaultValue with value prop: value={dttmToDayjs(sinceDatetime)}
</details>
###### Chat with Korbit by mentioning @korbit-ai, and give a 👍 or 👎 to help
Korbit improve your reviews.
<!--- korbi internal id:4e0ecebb-f0b8-4fb7-816f-d5d5200b601a -->
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
