mistercrunch commented on code in PR #31019:
URL: https://github.com/apache/superset/pull/31019#discussion_r1917151826
##########
superset-frontend/src/components/ListView/Filters/DateRange.tsx:
##########
@@ -51,40 +54,47 @@ function DateRangeFilter(
ref: RefObject<FilterHandler>,
) {
const [value, setValue] = useState<ValueState | null>(initialValue ?? null);
- const momentValue = useMemo((): [Moment, Moment] | null => {
+ const dayjsValue = useMemo((): [Dayjs, Dayjs] | null => {
if (!value || (Array.isArray(value) && !value.length)) return null;
- return [moment(value[0]), moment(value[1])];
+ return [extendedDayjs(value[0]), extendedDayjs(value[1])];
}, [value]);
+ const locale = useLocale();
+
useImperativeHandle(ref, () => ({
clearFilter: () => {
setValue(null);
onSubmit([]);
},
}));
+ if (locale === null) {
+ return <Loading position="inline-centered" />;
+ }
return (
- <RangeFilterContainer>
- <FormLabel>{Header}</FormLabel>
- <RangePicker
- placeholder={[t('Start date'), t('End date')]}
- showTime
- value={momentValue}
- onChange={momentRange => {
- if (!momentRange) {
- setValue(null);
- onSubmit([]);
- return;
- }
- const changeValue = [
- momentRange[0]?.valueOf() ?? 0,
- momentRange[1]?.valueOf() ?? 0,
- ] as ValueState;
- setValue(changeValue);
- onSubmit(changeValue);
- }}
- />
- </RangeFilterContainer>
+ <AntdThemeProvider locale={locale}>
Review Comment:
Yes, looking at what is needed here it seems it will fetch some local
configuration async and isn't ready to render until those locale files are
loaded, so we really don't want that at the base of the tree (aka in the main
ThemeProvider) and delay the first render. It's important to **not** hold up
the main tree which is what is happening here.
Now, knowing that it must be inject more dynamically or on-demand, it'd be
great for it to be DRY (single injection point if/where needed) and for the
fetch-locale logic to be cached to it only happens once in the SPA. I didn't
review the logic here to see whether there's any caching, but squeezing it in
the client-side cache somewhere seems important.
Now how should the components that need it get it? Maybe some sort of
reusable AntdLocaleProvider decorator or something simple like that, where
components that need would just wrap themselves with it (?) That wrapper itself
could take care of the caching with some way to share that `locale` object with
other instances of that same component.
--
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]
