-
Notifications
You must be signed in to change notification settings - Fork 8.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ResponseOps] [Cases] UX enhancements #146608
Conversation
Pinging @elastic/response-ops (Team:ResponseOps) |
Pinging @elastic/response-ops-cases (Feature:Cases) |
@@ -22,7 +21,7 @@ export const schema: FormSchema<AddCommentFormSchema> = { | |||
type: FIELD_TYPES.TEXTAREA, | |||
validations: [ | |||
{ | |||
validator: emptyField(i18n.COMMENT_REQUIRED), | |||
validator: emptyField(''), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Do we still need this schema file since we're not showing any errors anymore?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't display the error but I thought some frontend validation wouldn't harm. 😅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The emptyField
validator will throw an error if the comment contains only empty characters. The current behavior is to show an error message to the user in this scenario. I think we should keep showing the error and change the error message (only for the above scenario) to something more descriptive like "Empty comment is not allowed". Another alternative is to disable the button if the comment contains only empty characters but I found it a bit confusing that I cannot press the button even though I typed something. The error message will inform me of the requirements.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah I agree with @cnasikas. I think what Michael was getting at in the issue was to not show an error if the user hasn't typed anything yet. So if there's an easy way to differentiate between hasn't typed vs typed all spaces we should show an error if it's all spaces but just disable the button if the user hasn't typed.
…nt in the Case detail page.
handleSaveAction, | ||
handleCancelAction, | ||
}) => { | ||
const [{ content }] = useFormData<{ content: string }>({ watch: [fieldName] }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: Is a bit confusing that we pass a variable for the field but the type and the destructed value are known. Maybe we can remove the fieldName
prop and hardcode the field name to content
💚 Build Succeeded
Metrics [docs]Module Count
Async chunks
Page load bundle
Unknown metric groupsESLint disabled in files
ESLint disabled line counts
Total ESLint disabled count
History
To update your PR or re-run it, just comment with: |
Fixes #144614
Summary
Case Detail Page
Case Creation Page
All Cases list
Screenshots
Disabled Add comment button
Enabled Add comment button
Hollow tags
Fixed sidebar spacing
Create and Cancel button spacing
Cancel create case confirmation dialog
Centered, restricted page width
Margin between tags