alexandrusoare commented on code in PR #32729:
URL: https://github.com/apache/superset/pull/32729#discussion_r2005107003
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/ColumnSelect.tsx:
##########
@@ -25,16 +25,17 @@ import {
useChangeEffect,
getClientErrorObject,
} from '@superset-ui/core';
-import { Select, FormInstance } from 'src/components';
+import { Select } from 'src/components';
+import { FormInstance } from 'src/components/Form/Form';
import { useToasts } from 'src/components/MessageToasts/withToasts';
Review Comment:
Maybe it would be better if we would handle everything in the `index.tsx` of
`src/components/Form` so that we won't have to import like `Form/Form` . I've
seen that's the common pattern. What do you think?
##########
superset-frontend/src/components/Form/Form.tsx:
##########
@@ -16,21 +16,19 @@
* specific language governing permissions and limitations
* under the License.
*/
-// eslint-disable-next-line no-restricted-imports
-import AntdForm, { FormProps } from 'antd/lib/form'; // TODO: Remove antd
-import { styled } from '@superset-ui/core';
+import { Form as AntdForm } from 'antd-v5';
+import { FormProps, FormInstance, FormItemProps } from 'antd-v5/es/form';
-const StyledForm = styled(AntdForm)`
- &.ant-form label {
- font-size: ${({ theme }) => theme.fontSizeSM}px;
- }
- .ant-form-item {
- margin-bottom: ${({ theme }) => theme.sizeUnit * 4}px;
- }
-`;
-
-export default function Form(props: FormProps) {
- return <StyledForm {...props} />;
+function Form(props: FormProps) {
+ return <AntdForm {...props} />;
}
-export type { FormProps };
+export default Object.assign(Form, {
+ useForm: AntdForm.useForm,
+ Item: AntdForm.Item,
+ List: AntdForm.List,
+ ErrorList: AntdForm.ErrorList,
+ Provider: AntdForm.Provider,
+});
+
Review Comment:
I think this can be reduced to this => `const Form = Object.assign(
(props: FormProps) => <AntdForm {...props} />,
AntdForm,
) as typeof AntdForm;`
##########
superset-frontend/src/dashboard/components/nativeFilters/FiltersConfigModal/FiltersConfigForm/FilterScope/FilterScope.tsx:
##########
@@ -42,7 +42,7 @@ const Wrapper = styled.div`
padding: 0px ${({ theme }) => theme.sizeUnit * 4}px;
`;
-const CleanFormItem = styled(AntdForm.Item)`
+const CleanFormItem = styled(Form.Item)`
margin-bottom: 0;
Review Comment:
I haven't checked but if the only the `FormItem is used here, then maybe we
can import the FormItem directly instead of the whole form 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]
