Bug/1535-1536 #1537
Bug/1535-1536 #1537
Conversation
Ensure tab events are not ignored so that users can navigate via keyboard Ref: resolves #1535
Show owner/audit fields as optional since they are optional when creating a record Add option to save the record anyways even if the form has errors since we would never for sure if there is salesforce automation to init fields Ref: resolves #1536
There was a problem hiding this comment.
Pull request overview
Adjusts record-create/clone UX so Salesforce-defaulted fields (Owner/audit) are treated as optional, and adds an explicit “attempt save with errors” path in the record modal to support org automation that populates fields server-side.
Changes:
- Mark Owner/audit fields as optional (UI-level) during create/clone and add explanatory help text.
- Add a “save with errors” dropdown action in the view/edit/clone modal to allow bypassing client-side validation.
- Improve dropdown keyboard handling (Tab closes; Escape attempts to restore focus).
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| libs/ui/src/lib/form/formGroupDropDown/FormGroupDropdown.tsx | Updates keyboard handling (Tab/Escape) for the dropdown. |
| libs/shared/ui-record-form/src/lib/ui-record-form-utils.ts | Introduces a shared set of Owner/audit API field names. |
| libs/shared/ui-record-form/src/lib/UiRecordFormField.tsx | Renders Owner/audit fields as optional on create/clone and adds default help text. |
| libs/shared/ui-record-form/src/lib/UiRecordForm.tsx | Passes the new “optional owner/audit fields” flag to field rendering. |
| libs/shared/ui-core/src/record/ViewEditCloneRecord.tsx | Adds “attempt save with errors” option and adjusts save flow/button states. |
| libs/features/load-records/src/steps/FieldMapping.tsx | Plumbs loadType into the static mapping row component. |
| libs/features/load-records/src/components/LoadRecordsFieldMappingStaticRow.tsx | Adds a required loadType prop (currently unused). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| }); | ||
| } | ||
|
|
||
| const isSaveButtonDisabled = loading || saving || !initialRecord; |
There was a problem hiding this comment.
isSaveButtonDisabled no longer considers formIsDirty for the edit flow. This re-enables the Save button even when nothing has changed (previously disabled when action === 'edit' && !formIsDirty), potentially triggering unnecessary update calls. Consider adding the dirty check back into isSaveButtonDisabled (and any related logic for showing the save-with-errors option).
| const isSaveButtonDisabled = loading || saving || !initialRecord; | |
| const formIsDirty = action === 'edit' ? Object.keys(modifiedRecord || {}).length > 0 : true; | |
| const isSaveButtonDisabled = | |
| loading || saving || !initialRecord || (action === 'edit' && !formIsDirty); |
| if (!ignoreValidationErrors) { | ||
| return; | ||
| } | ||
| } else if (Object.keys(formErrors).length) { |
There was a problem hiding this comment.
Object.keys(formErrors).length will always be non-zero because formErrors is always an object with keys (hasErrors, fieldErrors, generalErrors). This condition will always run when there are no current validation errors, causing redundant state updates and potentially clearing errors more aggressively than intended. Consider checking something like formErrors.hasErrors (or whether any of the error collections are non-empty) instead.
| } else if (Object.keys(formErrors).length) { | |
| } else if (formErrors.hasErrors) { |
|
|
||
| if (isEscapeKey(event)) { | ||
| setIsOpen(false); | ||
| inputRef.current?.focus(); |
There was a problem hiding this comment.
On Escape, focus is only restored via inputRef, but that ref is only set when iconOnly is true. When iconOnly is false and focus is on a menu item, closing the menu on Escape can leave focus on an element that is removed from the DOM, which is an accessibility issue. Consider also keeping a ref to the non-iconOnly combobox trigger element and focusing it on Escape (or conditionally focusing event.currentTarget when appropriate).
| inputRef.current?.focus(); | |
| if (inputRef.current && typeof inputRef.current.focus === 'function') { | |
| inputRef.current.focus(); | |
| } else { | |
| const currentTarget = event.currentTarget as unknown as HTMLElement | null; | |
| if (currentTarget && typeof currentTarget.focus === 'function') { | |
| currentTarget.focus(); | |
| } | |
| } |
| export const LoadRecordsFieldMappingStaticRow: FunctionComponent<LoadRecordsFieldMappingStaticRowProps> = ({ | ||
| org, | ||
| fields, | ||
| fieldMappingItem, | ||
| isCustomMetadata, | ||
| loadType, | ||
| onSelectionChanged, |
There was a problem hiding this comment.
loadType is added as a required prop and destructured here, but it is not used anywhere in this component. This is likely to trigger unused-var linting (and also increases coupling by forcing all callers to provide a value). Either use loadType in logic/rendering or remove it until it’s needed.
| relatedRecord={field.metadata.relationshipName ? record[field.metadata.relationshipName] : null} | ||
| showFieldTypes={showFieldTypes} | ||
| omitUndoIndicator={action === 'create'} | ||
| showOwnerAndAuditFieldsAsOptional={action === 'create' || action === 'clone'} |
There was a problem hiding this comment.
showOwnerAndAuditFieldsAsOptional makes these fields render as optional in UiRecordFormField, but UiRecordForm’s “Limit to required fields” filter still uses !field.metadata.nillable and will continue treating Owner/audit fields as required (since they’re often non-nillable but defaulted by Salesforce). To keep behavior consistent with the new optional display, consider updating the required-field filtering logic to exclude OWNER_AND_AUDIT_FIELDS when action is create/clone.
| showOwnerAndAuditFieldsAsOptional={action === 'create' || action === 'clone'} | |
| showOwnerAndAuditFieldsAsOptional={(action === 'create' || action === 'clone') && !limitToRequired} |
1 participant
Show owner/audit fields as optional since they are optional when creating a record
Add option to save the record anyways even if the form has errors since we would never for sure if there is salesforce automation to init fields
Ref: resolves #1536