You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@superset.apache.org by GitBox <gi...@apache.org> on 2021/05/07 19:23:10 UTC
[GitHub] [superset] lyndsiWilliams opened a new pull request #14530: feat: Labeled Error-bound Input
lyndsiWilliams opened a new pull request #14530:
URL: https://github.com/apache/superset/pull/14530
### SUMMARY
I have created a new dynamic error-bound input. When the user interacts with this form, if an error occurs (designated by a function passed into the `validationMethods` parameter), the error will replace any existing helper text until the error is resolved. `<LabeledErrorBoundInput />` accepts the following parameters in order to customize the input:
```label?: string;
name: string;
validationMethods:
| { onBlur: (value: any) => void }
| { onChange: (value: any) => void };
errorMessage: string | null;
helpText?: string;
value: string | number | readonly string[] | undefined;
required?: boolean;
placeholder?: string;
autocomplete?: string;
type?: string;
id?: string;
```
### ANIMATED GIF
### TEST PLAN
- Run Storybook
- Click Form > Interactive Form
- Type success into the input to observe error behavior
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [x] Introduces new feature or API
- [ ] Removes existing feature or API
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r632789022
##########
File path: superset-frontend/src/components/Form/FormLabel.tsx
##########
@@ -40,6 +40,7 @@ const RequiredLabel = styled.label`
margin-bottom: ${({ theme }) => theme.gridUnit}px;
&::after {
display: inline-block;
+ margin-left: 4px;
Review comment:
Good catch! Fixed at [`this commit`](https://github.com/apache/superset/pull/14530/commits/16c52e443ce21981aa2312c3b773cfc6312566c9)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r631886847
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
+ type={type}
Review comment:
Also resolved at [`this commit`](https://github.com/apache/superset/pull/14530/commits/da526cbb42275327b24cffa888ea43087c13e82d)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633666748
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
+ id?: string;
+ onChange?: any;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css`
+ .ant-form-item-children-icon {
+ display: none;
+ }
+ ${hasError &&
+ `.ant-form-item-control-input-content {
+ position: relative;
+
+ &:after {
+ content: ' ';
+ display: inline-block;
+ background: ${theme.colors.error.base};
+ mask: url('/images/icons/error.svg');
+ mask-size: cover;
+ width: ${theme.gridUnit * 4}px;
+ height: ${theme.gridUnit * 4}px;
+ position: absolute;
+ right: 7px;
+ top: 15px;
+ }
+ }`}
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ validationMethods,
+ errorMessage,
+ helpText,
+ required = false,
+ id,
+ ...props
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel htmlFor={id} required={required}>
+ {label}
+ </FormLabel>
+ <FormItem
+ css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ hasFeedback={!!errorMessage}
+ {...props}
Review comment:
remind me, did we say that we were going to spread the props into the input first and then let people take them out explicitly if they needed them here?
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
Review comment:
I don't think this is being used below. You can remove it, and let it pass through in the props spread instead.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633924901
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
+ id?: string;
+ onChange?: any;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css`
+ .ant-form-item-children-icon {
+ display: none;
+ }
+ ${hasError &&
+ `.ant-form-item-control-input-content {
+ position: relative;
+
+ &:after {
+ content: ' ';
+ display: inline-block;
+ background: ${theme.colors.error.base};
+ mask: url('/images/icons/error.svg');
+ mask-size: cover;
+ width: ${theme.gridUnit * 4}px;
+ height: ${theme.gridUnit * 4}px;
+ position: absolute;
+ right: 7px;
+ top: 15px;
+ }
+ }`}
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ validationMethods,
+ errorMessage,
+ helpText,
+ required = false,
+ id,
+ ...props
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel htmlFor={id} required={required}>
+ {label}
+ </FormLabel>
+ <FormItem
+ css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ hasFeedback={!!errorMessage}
+ {...props}
Review comment:
Oh you're right! Fixed it [`here`](https://github.com/apache/superset/pull/14530/commits/f476e27f08885c461b448c6b6042f5d149ec1935#diff-97dbad737572defaaa9f77fd37e305d4ef4080680a20708c7018dac7c47161b0L88)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f476e27) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head f476e27 differs from pull request most recent head fb6fe2f. Consider uploading reports for the commit fb6fe2f to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.43% +0.02%
==========================================
Files 958 958
Lines 48329 48390 +61
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10720 +1
Partials 200 200
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.54% <94.44%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [...rontend/src/components/ListView/Filters/Search.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvRmlsdGVycy9TZWFyY2gudHN4) | `82.35% <0.00%> (-4.32%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| ... and [43 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...fb6fe2f](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r631911406
##########
File path: superset-frontend/src/components/Form/Form.stories.tsx
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState } from 'react';
+import LabeledErrorBoundInput, { Form } from '.';
+
+export default {
+ title: 'Form',
+ component: Form,
+};
+
+export const InteractiveForm = () => {
+ const [checkErrorMessage, setCheckErrorMessage] = useState('');
+
+ const validateFunctionality: (value: any) => void = value => {
+ if (value.target.value.includes('success')) {
+ setCheckErrorMessage('');
+ } else {
+ setCheckErrorMessage('Type success in the text bar');
+ }
+ };
+
+ return (
+ <LabeledErrorBoundInput
Review comment:
And [`this one`](https://github.com/apache/superset/pull/14530/commits/f46dc8841865dbd9deb91ef93112f6833d78ce3f)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] commented on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] commented on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7d99d3d) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.43% +0.02%
==========================================
Files 958 958
Lines 48329 48392 +63
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10721 +2
- Partials 200 201 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.53% <100.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `68.75% <ø> (-31.25%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| [...ontend/src/dashboard/components/dnd/handleHover.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVIb3Zlci5qcw==) | `9.09% <0.00%> (-2.03%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...7d99d3d](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r631892695
##########
File path: superset-frontend/src/components/Form/Form.stories.tsx
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState } from 'react';
+import LabeledErrorBoundInput, { Form } from '.';
+
+export default {
+ title: 'Form',
+ component: Form,
+};
+
+export const InteractiveForm = () => {
+ const [checkErrorMessage, setCheckErrorMessage] = useState('');
+
+ const validateFunctionality: (value: any) => void = value => {
+ if (value.target.value.includes('success')) {
+ setCheckErrorMessage('');
+ } else {
+ setCheckErrorMessage('Type success in the text bar');
+ }
+ };
+
+ return (
+ <LabeledErrorBoundInput
Review comment:
Fixed at [`this commit`](https://github.com/apache/superset/pull/14530/commits/7f2e7534091d695a41f75cd1e7dc9663acd5c8e2)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] AAfghahi commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
AAfghahi commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r632755649
##########
File path: superset-frontend/src/components/Form/FormLabel.tsx
##########
@@ -40,6 +40,7 @@ const RequiredLabel = styled.label`
margin-bottom: ${({ theme }) => theme.gridUnit}px;
&::after {
display: inline-block;
+ margin-left: 4px;
Review comment:
I think using grid unit here like you margin-bottom is preferred.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633899529
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
Review comment:
we can replace the type with a catchall: `[x: string]: any`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633668411
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
Review comment:
I don't think this is being used below. You can remove it, and let it pass through in the props spread instead.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r629489600
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
+ type={type}
Review comment:
same with this. basically anything that is for a regular html input element can be passed through. which actually makes me think now that some may need to go on the input instead.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16c52e4) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `94.44%`.
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.42% +0.02%
==========================================
Files 958 958
Lines 48329 48393 +64
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10722 +3
- Partials 200 201 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.53% <94.44%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| [...ontend/src/dashboard/components/dnd/handleHover.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVIb3Zlci5qcw==) | `9.09% <0.00%> (-2.03%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...16c52e4](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r629489600
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
+ type={type}
Review comment:
same with this
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida merged pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
betodealmeida merged pull request #14530:
URL: https://github.com/apache/superset/pull/14530
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r629489453
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
Review comment:
I would suggest removing this and then just passing through any extra props that you get.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r629489600
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
+ type={type}
Review comment:
same with this. basically anything that is for a regular html input element can be passed through.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r634044554
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx
##########
@@ -33,24 +33,24 @@ export const InteractiveLabeledErrorBoundInput = ({
type,
id,
}: LabeledErrorBoundInputProps) => {
- const [checkErrorMessage, setCheckErrorMessage] = useState('');
const [currentValue, setCurrentValue] = useState(value);
- const validateFunctionality: (value: any) => void = value => {
+ const validateFunctionality: (value: any) => string = value => {
setCurrentValue(value.target.value);
if (value.target.value.includes('success')) {
- setCheckErrorMessage('');
- } else {
- setCheckErrorMessage('Type success in the text bar');
+ return 'success';
}
+ return 'error';
};
Review comment:
My suggestion was slightly different, having no state at all in the validator:
```js
const validateFunctionality: (value: any) => (string | void) = value => {
if not (value.target.value.includes('success')) {
return 'Type success in the text bar';
}
};
```
Basically, if there's an error return an error message. If the validation is good, return nothing (void, null, undefined, etc.). The function has no side-effects and uses no external hooks.
This way, we can implement reusable validators:
```js
const isInteger: (value: any) => (string | void) = value => {
const reg = new RegExp('^\\d+$');
if not reg.match(value) {
return 'Number is not an integer';
}
};
```
Then, in the `LabeledErrorBoundInput` component we'd wrap the validators in a function that sets the error message. Something like this pseudocode:
```js
const LabeledErrorBoundInput = ({
label,
validationMethods,
//errorMessage, // remove this, we'll read it from the validation methods
helpText,
required = false,
id,
...props
}: LabeledErrorBoundInputProps) => {
const [errorMessage, setErrorMessage] = useState('');
// wrap validators in a function that sets the error message
const validators = {};
for (const [trigger, validator] of Object.entries(validationMethods)) {
validators[trigger] = function(value) {
const message = validator(value);
if (message) {
setErrorMessage(message);
}
};
}
return (
<>
<FormLabel htmlFor={id} required={required}>
{label}
</FormLabel>
<FormItem
css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
validateTrigger={Object.keys(validators)}
validateStatus={errorMessage ? 'error' : 'success'}
help={errorMessage || helpText}
hasFeedback={!!errorMessage}
>
<StyledInput {...props} {...validators} />
</FormItem>
</>
)
};
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r629487904
##########
File path: superset-frontend/src/components/Form/Form.stories.tsx
##########
@@ -0,0 +1,49 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React, { useState } from 'react';
+import LabeledErrorBoundInput, { Form } from '.';
+
+export default {
+ title: 'Form',
+ component: Form,
+};
+
+export const InteractiveForm = () => {
+ const [checkErrorMessage, setCheckErrorMessage] = useState('');
+
+ const validateFunctionality: (value: any) => void = value => {
+ if (value.target.value.includes('success')) {
+ setCheckErrorMessage('');
+ } else {
+ setCheckErrorMessage('Type success in the text bar');
+ }
+ };
+
+ return (
+ <LabeledErrorBoundInput
Review comment:
I would suggest renaming the file to this since we changed the naming of the 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633899529
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
Review comment:
we can replace it with a catchall: `[x: string]: any`
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633925187
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
Review comment:
Much cleaner! Fixed in [`this commit`](https://github.com/apache/superset/pull/14530/commits/f476e27f08885c461b448c6b6042f5d149ec1935)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-842715627
A bit late on this, but one useful pattern would be to have the validators return `null` (if no errors) or an error message (if any errors). Then the validators don't need to worry about updating state.
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] lyndsiWilliams commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
lyndsiWilliams commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r631886688
##########
File path: superset-frontend/src/components/Form/index.tsx
##########
@@ -16,8 +16,64 @@
* specific language governing permissions and limitations
* under the License.
*/
+import React from 'react';
+import { Input } from 'antd';
+import { styled } from '@superset-ui/core';
import Form from './Form';
import FormItem from './FormItem';
import FormLabel from './FormLabel';
+interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autocomplete?: string;
+ type?: string;
+ id?: string;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ name,
+ validationMethods,
+ errorMessage,
+ helpText,
+ value,
+ required = false,
+ placeholder,
+ autocomplete,
+ type,
+ id,
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel required={required}>{label}</FormLabel>
+ <FormItem
+ name={name}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ id={id}
+ placeholder={placeholder}
+ autocomplete={autocomplete}
Review comment:
Much cleaner! Resolved at [`this commit`](https://github.com/apache/superset/pull/14530/commits/da526cbb42275327b24cffa888ea43087c13e82d)
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (16c52e4) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `94.44%`.
> :exclamation: Current head 16c52e4 differs from pull request most recent head f476e27. Consider uploading reports for the commit f476e27 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.42% +0.02%
==========================================
Files 958 958
Lines 48329 48393 +64
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10722 +3
- Partials 200 201 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.53% <94.44%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| [...ontend/src/dashboard/components/dnd/handleHover.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVIb3Zlci5qcw==) | `9.09% <0.00%> (-2.03%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...f476e27](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] betodealmeida commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
betodealmeida commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r634044554
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.stories.tsx
##########
@@ -33,24 +33,24 @@ export const InteractiveLabeledErrorBoundInput = ({
type,
id,
}: LabeledErrorBoundInputProps) => {
- const [checkErrorMessage, setCheckErrorMessage] = useState('');
const [currentValue, setCurrentValue] = useState(value);
- const validateFunctionality: (value: any) => void = value => {
+ const validateFunctionality: (value: any) => string = value => {
setCurrentValue(value.target.value);
if (value.target.value.includes('success')) {
- setCheckErrorMessage('');
- } else {
- setCheckErrorMessage('Type success in the text bar');
+ return 'success';
}
+ return 'error';
};
Review comment:
My suggestion was slightly different, having no state at all in the validator:
```js
const validateFunctionality: (value: any) => (string | void) = value => {
if not (value.target.value.includes('success')) {
return 'Type success in the text bar';
}
};
```
Basically, if there's an error return an error message. If the validation is good, return nothing (void, null, undefined, etc.). The function has no side-effects and uses no external hooks.
This way, we can implement reusable validators:
```js
const isInteger: (value: any) => (string | void) = value => {
const reg = new RegExp('^\\d+$');
if not reg.match(value) {
return 'Number is not an integer';
}
};
```
Then, in the `LabeledErrorBoundInput` component we'd wrap the validators in a function that sets the error message. Something like (sorry for not using types for writing old-school JS):
```js
const LabeledErrorBoundInput = ({
label,
validationMethods,
//errorMessage, // remove this, we'll read it from the validation methods
helpText,
required = false,
id,
...props
}: LabeledErrorBoundInputProps) => {
const [errorMessage, setErrorMessage] = useState('');
// wrap validators in a function that sets the error message
const validators = {};
for (const [trigger, validator] of Object.entries(validationMethods)) {
validators[trigger] = function(value) {
const message = validator(value);
if (message) {
setErrorMessage(message);
}
};
}
return (
<>
<FormLabel htmlFor={id} required={required}>
{label}
</FormLabel>
<FormItem
css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
validateTrigger={Object.keys(validators)}
validateStatus={errorMessage ? 'error' : 'success'}
help={errorMessage || helpText}
hasFeedback={!!errorMessage}
>
<StyledInput {...props} {...validators} />
</FormItem>
</>
)
};
```
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (f476e27) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `94.44%`.
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.43% +0.02%
==========================================
Files 958 958
Lines 48329 48390 +61
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10720 +1
Partials 200 200
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.54% <94.44%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [...rontend/src/components/ListView/Filters/Search.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGlzdFZpZXcvRmlsdGVycy9TZWFyY2gudHN4) | `82.35% <0.00%> (-4.32%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| ... and [43 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...f476e27](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] eschutho commented on a change in pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
eschutho commented on a change in pull request #14530:
URL: https://github.com/apache/superset/pull/14530#discussion_r633666748
##########
File path: superset-frontend/src/components/Form/LabeledErrorBoundInput.tsx
##########
@@ -0,0 +1,95 @@
+/**
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+import React from 'react';
+import { Input } from 'antd';
+import { styled, css, SupersetTheme } from '@superset-ui/core';
+import FormItem from './FormItem';
+import FormLabel from './FormLabel';
+
+export interface LabeledErrorBoundInputProps {
+ label?: string;
+ name: string;
+ validationMethods:
+ | { onBlur: (value: any) => void }
+ | { onChange: (value: any) => void };
+ errorMessage: string | null;
+ helpText?: string;
+ value: string | number | readonly string[] | undefined;
+ required?: boolean;
+ placeholder?: string;
+ autoComplete?: string;
+ type?: string;
+ id?: string;
+ onChange?: any;
+}
+
+const StyledInput = styled(Input)`
+ margin: 8px 0;
+`;
+
+const alertIconStyles = (theme: SupersetTheme, hasError: boolean) => css`
+ .ant-form-item-children-icon {
+ display: none;
+ }
+ ${hasError &&
+ `.ant-form-item-control-input-content {
+ position: relative;
+
+ &:after {
+ content: ' ';
+ display: inline-block;
+ background: ${theme.colors.error.base};
+ mask: url('/images/icons/error.svg');
+ mask-size: cover;
+ width: ${theme.gridUnit * 4}px;
+ height: ${theme.gridUnit * 4}px;
+ position: absolute;
+ right: 7px;
+ top: 15px;
+ }
+ }`}
+`;
+
+const LabeledErrorBoundInput = ({
+ label,
+ validationMethods,
+ errorMessage,
+ helpText,
+ required = false,
+ id,
+ ...props
+}: LabeledErrorBoundInputProps) => (
+ <>
+ <FormLabel htmlFor={id} required={required}>
+ {label}
+ </FormLabel>
+ <FormItem
+ css={(theme: SupersetTheme) => alertIconStyles(theme, !!errorMessage)}
+ validateTrigger={Object.keys(validationMethods)}
+ validateStatus={errorMessage ? 'error' : 'success'}
+ help={errorMessage || helpText}
+ hasFeedback={!!errorMessage}
+ {...props}
Review comment:
remind me, did we say that we were going to spread the props into the input first and then let people take them out explicitly if they needed them here?
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (7d99d3d) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.02%`.
> The diff coverage is `100.00%`.
> :exclamation: Current head 7d99d3d differs from pull request most recent head 16c52e4. Consider uploading reports for the commit 16c52e4 to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.43% +0.02%
==========================================
Files 958 958
Lines 48329 48392 +63
Branches 5679 5709 +30
==========================================
+ Hits 37410 37470 +60
- Misses 10719 10721 +2
- Partials 200 201 +1
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.53% <100.00%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `68.75% <ø> (-31.25%)` | :arrow_down: |
| [...end/src/components/Form/LabeledErrorBoundInput.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9MYWJlbGVkRXJyb3JCb3VuZElucHV0LnRzeA==) | `100.00% <100.00%> (ø)` | |
| [superset-frontend/src/components/Form/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9pbmRleC50c3g=) | `100.00% <100.00%> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...ponents/controls/TimeSeriesColumnControl/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9UaW1lU2VyaWVzQ29sdW1uQ29udHJvbC9pbmRleC5qc3g=) | `91.78% <0.00%> (-8.22%)` | :arrow_down: |
| [...src/dashboard/components/dnd/handleScroll/index.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVTY3JvbGwvaW5kZXgudHM=) | `65.21% <0.00%> (-8.12%)` | :arrow_down: |
| [superset-frontend/src/CRUD/Field.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGQuanN4) | `80.00% <0.00%> (-3.34%)` | :arrow_down: |
| [...trol/AdhocFilterEditPopoverSqlTabContent/index.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2V4cGxvcmUvY29tcG9uZW50cy9jb250cm9scy9GaWx0ZXJDb250cm9sL0FkaG9jRmlsdGVyRWRpdFBvcG92ZXJTcWxUYWJDb250ZW50L2luZGV4LmpzeA==) | `61.29% <0.00%> (-2.05%)` | :arrow_down: |
| [...ontend/src/dashboard/components/dnd/handleHover.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9jb21wb25lbnRzL2RuZC9oYW5kbGVIb3Zlci5qcw==) | `9.09% <0.00%> (-2.03%)` | :arrow_down: |
| ... and [34 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...16c52e4](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org
[GitHub] [superset] codecov[bot] edited a comment on pull request #14530: feat: Labeled Error-bound Input
Posted by GitBox <gi...@apache.org>.
codecov[bot] edited a comment on pull request #14530:
URL: https://github.com/apache/superset/pull/14530#issuecomment-841472099
# [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#14530](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4ed4c4f) into [master](https://codecov.io/gh/apache/superset/commit/6d786d4d47274864e8c0418c5a52789017bd879f?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (6d786d4) will **increase** coverage by `0.15%`.
> The diff coverage is `84.11%`.
> :exclamation: Current head 4ed4c4f differs from pull request most recent head fb6fe2f. Consider uploading reports for the commit fb6fe2f to get more accurate results
[](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
```diff
@@ Coverage Diff @@
## master #14530 +/- ##
==========================================
+ Coverage 77.40% 77.56% +0.15%
==========================================
Files 958 959 +1
Lines 48329 48568 +239
Branches 5679 5709 +30
==========================================
+ Hits 37410 37671 +261
+ Misses 10719 10697 -22
Partials 200 200
```
| Flag | Coverage Δ | |
|---|---|---|
| javascript | `72.54% <80.14%> (+0.06%)` | :arrow_up: |
Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#carryforward-flags-in-the-pull-request-comment) to find out more.
| [Impacted Files](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [superset-frontend/src/CRUD/Fieldset.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL0NSVUQvRmllbGRzZXQuanN4) | `85.71% <ø> (ø)` | |
| [superset-frontend/src/SqlLab/actions/sqlLab.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9hY3Rpb25zL3NxbExhYi5qcw==) | `58.97% <ø> (ø)` | |
| [...et-frontend/src/SqlLab/components/TableElement.jsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL1NxbExhYi9jb21wb25lbnRzL1RhYmxlRWxlbWVudC5qc3g=) | `89.02% <ø> (ø)` | |
| [...ontend/src/common/hooks/apiResources/dashboards.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbW1vbi9ob29rcy9hcGlSZXNvdXJjZXMvZGFzaGJvYXJkcy50cw==) | `40.00% <0.00%> (-10.00%)` | :arrow_down: |
| [...et-frontend/src/components/EditableTitle/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRWRpdGFibGVUaXRsZS9pbmRleC50c3g=) | `83.95% <ø> (ø)` | |
| [...rset-frontend/src/components/ErrorMessage/types.ts](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRXJyb3JNZXNzYWdlL3R5cGVzLnRz) | `100.00% <ø> (ø)` | |
| [...uperset-frontend/src/components/Form/FormLabel.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvRm9ybS9Gb3JtTGFiZWwudHN4) | `64.70% <0.00%> (-35.30%)` | :arrow_down: |
| [superset-frontend/src/components/Label/index.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvTGFiZWwvaW5kZXgudHN4) | `100.00% <ø> (ø)` | |
| [superset-frontend/src/components/Select/styles.tsx](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2NvbXBvbmVudHMvU2VsZWN0L3N0eWxlcy50c3g=) | `84.72% <ø> (ø)` | |
| [superset-frontend/src/dashboard/actions/hydrate.js](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3VwZXJzZXQtZnJvbnRlbmQvc3JjL2Rhc2hib2FyZC9hY3Rpb25zL2h5ZHJhdGUuanM=) | `1.73% <0.00%> (-0.02%)` | :arrow_down: |
| ... and [94 more](https://codecov.io/gh/apache/superset/pull/14530/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [6d786d4...fb6fe2f](https://codecov.io/gh/apache/superset/pull/14530?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@superset.apache.org
For additional commands, e-mail: notifications-help@superset.apache.org