Added isInvalid styles for forms #935
Conversation
cmd/wire_gen.go
Outdated
| /* | ||
| * 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. | ||
| */ |
There was a problem hiding this comment.
This header should not be deleted. You can use the make lint command.
There was a problem hiding this comment.
Okay. Will add these headers ...
There was a problem hiding this comment.
Also, the pnpm-lock.yaml file has lots of changes when I try to build UI using pnpm build. I'm using the same pnpm version 8.9.2.
| @@ -83,6 +83,7 @@ const Index = ({ | |||
| size="sm" | |||
| value={type === 'edit' ? parseEditMentionUser(value) : value} | |||
| onChange={handleChange} | |||
| isInvalid={validationErrorMsg !== ''} | |||
There was a problem hiding this comment.
ui/src/components/Comment/components/Reply/index.tsx, this file also needs to be adjusted!
| defaultValue={JSON.stringify(formData.tags.value)} | ||
| isInvalid={formData.tags.isInvalid} | ||
| hidden | ||
| /> |
There was a problem hiding this comment.
Correct error message：
the tag error message is not displayed,there are several solutions:
- TagSelector component adds
custom-form-control is-invalidclass name
- extend the component, put the error information of the component inside, and pass in the error information and status from props!
I prefer the second option！
There was a problem hiding this comment.
I got the issue that if classes custom-form-control and is-invalid both are not present in the child component of <Form.Group> component, the <Form.Control.Feedback> tag will not work. That's why it was not working earlier even though i passed is-invalid class because the is-invalid class was assign to its child's child component and <Form.Control.Feedback> has not been validated.
But I didn't completely get your second approach.
Should I suppose to do it this way
<Form.Label>{t('form.fields.tags.label')}</Form.Label>
<TagSelector
value={formData.tags.value}
onChange={handleTagsChange}
showRequiredTag
maxTagLength={5}
formData.tags
}
/>
<Form.Control.Feedback type="invalid">
{formData.tags.errorMsg}
</Form.Control.Feedback>
</Form.Group>
And then handle the error conditions inside TagSelector component?
I'm new to react, please help me to understand the expectations. Thanks you :)
There was a problem hiding this comment.
The cause of the problem is exactly as you described, so I say both solutions are possible.
In the second option, I just simplify the use of this component and build <Form.Control.Feedback /> into the TagSelector component.
eg:
usage:
There was a problem hiding this comment.
Understood!
Thanks for explaining @shuashuai.
But I have one concern about why we are putting <Form.Control.Feedback> component inside our custom <TagSelector>. Every component is following this structure ...
<Form.Group controlId="tags" className="my-3">
<Form.Label>title</Form.Label> // responsible for title of component
<CustomComponent/> // actual component
<Form.Control.Feedback type="invalid">
error // display error if any
</Form.Control.Feedback>
</Form.Group>
If we follow the second approach for <TagSelector>, then this structure is not followed. Should I make this change to all available components? Or, Can you tell me pros of following 2nd approach. Just want to get into details out of curiosity!
There was a problem hiding this comment.
This is indeed the case for a standard form structure as you mentioned.
I recommend the second option mainly from the perspective of ease of use of components, and unified management. From the first option, you can see that to combine this standard structure, we need to add additional attributes to the custom <Form. Control /> to conform to the standard structure. This may cause some confusion for some users who don’t know why.
For example, you may have found code like this:
To conform to the bootstrap structure, it is easy to write such confusing code. At the same time, this may be done for components that use custom form. control, if the custom form.control itself encapsulates the complete form function, it will greatly reduce the occurrence of this situation. it will also be easier to maintain.
The following structure will make the structure simpler and will not affect the reading of the code.
Of course, this may be more in line with the coding habits of our team, and I also want to change these unusual writing methods in later versions.
There was a problem hiding this comment.
Understood!
Made changes to files, please verify.
There was a problem hiding this comment.
/ui/src/pages/Review/components/EditPostModal/index.tsx , this file also needs to display error messages!
There was a problem hiding this comment.
Yes.
But I'm seeing blank page saying "No review tasks left". Can you show me exactly which component(screenshot would work) to fix to display error message.
There was a problem hiding this comment.
Sorry for taking so long to reply, but this is actually the same as the /questions/ask posed here, you can add this type of review by using the flag button below the issue details.
falg post:
form error:
This PR includes, fixing css style for some of the forms when clicked is invalid.
Issue:- #927
Question/ask page
Question/details page