feat: single-line-editor add an optional prop to hide all validation messages [LUMOS-312] #1805
Conversation
465bcf3 to
97ca4f6
Compare
97ca4f6 to
05bdb75
Compare
| /** | ||
| * whether validations should be rendered or not. | ||
| */ | ||
| validationsAreVisable?: boolean; |
There was a problem hiding this comment.
should we name it withValidations? to stay consistent in the naming
or withCharInformation? as the char counter would be no validation 🙈
There was a problem hiding this comment.
+1 to renaming to withCharInformation to match naming conventions of this component and it matches the purpose of the boolean a bit better since to Chris' point it's not just for validations but also the character counter.
| /** | ||
| * whether validations should be rendered or not. | ||
| */ | ||
| validationsAreVisable?: boolean; |
There was a problem hiding this comment.
+1 to renaming to withCharInformation to match naming conventions of this component and it matches the purpose of the boolean a bit better since to Chris' point it's not just for validations but also the character counter.
| <CharCounter value={value || ''} checkConstraint={() => true} /> | ||
| </div> | ||
| )} | ||
| </> |
There was a problem hiding this comment.
I think we can clean up these conditionals a bit to something like:
// new const for checkConstraint above the return of the component
const handleCheckConstraint = withCharValidation ? checkConstraint : () => true
// updated return
{withCharInformation && (
<div className={styles.validationRow}>
<CharCounter value={value || ''} checkConstraint={handleCheckConstraint} />
{withCharValidation && <CharValidation constraints={constraints} />}
</div>
)}There was a problem hiding this comment.
Nice, yeah this change definitely cleans the code up.
- clean up conditionals.
05bdb75 to
7861fbb
Compare
| @@ -229,4 +229,35 @@ describe('SingleLineEditor', () => { | |||
| expect(getByText('0 characters')).toBeInTheDocument(); | |||
| expect(queryByText('Requires between 100 and 1000 characters')).not.toBeInTheDocument(); | |||
| }); | |||
|
|
|||
| it('renders no validation message or counter if validationsAreVisable is false', () => { | |||
There was a problem hiding this comment.
NIT the test case is no longer correct after renaming the props 😅
✅ Closes: LUMOS-312
7861fbb to
f1c319a
Compare
Description
Add an optional prop to render all validation messages. This prop defaults to true, to ensure backward compatibility.
Necessary use case
In studio, users need to be able to bind component properties to entry/asset fields.
However, we do not allow users to make changes to the underlying entry/asset while they are in the process of binding (if they need to edit the underlying entry/asset, there is a separate form). Therefore, it doesn't make sense for studio to render the validation messages when binding.
with validation messages enabled
with withCharValidation = false
with validation messages disabled