-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix form record picker field #11817
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
base: main
Are you sure you want to change the base?
Fix form record picker field #11817
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -11,19 +11,30 @@ import { SingleRecordPickerRecord } from '@/object-record/record-picker/single-r | |
import { InputLabel } from '@/ui/input/components/InputLabel'; | ||
import { Dropdown } from '@/ui/layout/dropdown/components/Dropdown'; | ||
import { useDropdown } from '@/ui/layout/dropdown/hooks/useDropdown'; | ||
import { DropdownScope } from '@/ui/layout/dropdown/scopes/DropdownScope'; | ||
import { useSetRecoilComponentStateV2 } from '@/ui/utilities/state/component-state/hooks/useSetRecoilComponentStateV2'; | ||
import { isStandaloneVariableString } from '@/workflow/utils/isStandaloneVariableString'; | ||
import { useTheme } from '@emotion/react'; | ||
import styled from '@emotion/styled'; | ||
import { useCallback } from 'react'; | ||
import { isDefined, isValidUuid } from 'twenty-shared/utils'; | ||
import { IconChevronDown, IconForbid } from 'twenty-ui/display'; | ||
import { LightIconButton } from 'twenty-ui/input'; | ||
|
||
const StyledFormSelectContainer = styled(FormFieldInputInnerContainer)` | ||
justify-content: space-between; | ||
align-items: center; | ||
padding-right: ${({ theme }) => theme.spacing(1)}; | ||
padding-right: ${({ theme }) => theme.spacing(2)}; | ||
height: 32px; | ||
|
||
&:hover, | ||
&[data-open='true'] { | ||
background-color: ${({ theme }) => theme.background.transparent.light}; | ||
} | ||
|
||
cursor: pointer; | ||
`; | ||
|
||
const StyledIconButton = styled.div` | ||
display: flex; | ||
`; | ||
|
||
export type RecordId = string; | ||
|
@@ -58,6 +69,7 @@ export const FormSingleRecordPicker = ({ | |
testId, | ||
VariablePicker, | ||
}: FormSingleRecordPickerProps) => { | ||
const theme = useTheme(); | ||
const draftValue: FormSingleRecordPickerValue = isStandaloneVariableString( | ||
defaultValue, | ||
) | ||
|
@@ -103,12 +115,10 @@ export const FormSingleRecordPicker = ({ | |
|
||
const handleVariableTagInsert = (variable: string) => { | ||
onChange?.(variable); | ||
closeDropdown(); | ||
}; | ||
|
||
const handleUnlinkVariable = () => { | ||
closeDropdown(); | ||
|
||
const handleUnlinkVariable = (event?: React.MouseEvent<HTMLDivElement>) => { | ||
event?.stopPropagation(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For anyone reading this PR, the CleanShot.2025-04-30.at.18.02.35.mp4There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I feel there might be a less "hacky" solution to this. We might ask Lucas. |
||
onChange(''); | ||
}; | ||
|
||
|
@@ -130,47 +140,44 @@ export const FormSingleRecordPicker = ({ | |
<FormFieldInputContainer testId={testId}> | ||
{label ? <InputLabel>{label}</InputLabel> : null} | ||
<FormFieldInputRowContainer> | ||
<StyledFormSelectContainer | ||
hasRightElement={isDefined(VariablePicker) && !disabled} | ||
preventSetHotkeyScope={true} | ||
> | ||
<FormSingleRecordFieldChip | ||
draftValue={draftValue} | ||
selectedRecord={selectedRecord} | ||
objectNameSingular={objectNameSingular} | ||
onRemove={handleUnlinkVariable} | ||
disabled={disabled} | ||
/> | ||
{!disabled && ( | ||
<DropdownScope dropdownScopeId={dropdownId}> | ||
<Dropdown | ||
dropdownId={dropdownId} | ||
dropdownPlacement="left-start" | ||
onClose={handleCloseRelationPickerDropdown} | ||
onOpen={handleOpenDropdown} | ||
clickableComponent={ | ||
<LightIconButton | ||
className="displayOnHover" | ||
Icon={IconChevronDown} | ||
accent="tertiary" | ||
/> | ||
} | ||
dropdownComponents={ | ||
<SingleRecordPicker | ||
componentInstanceId={dropdownId} | ||
EmptyIcon={IconForbid} | ||
emptyLabel={'No ' + objectNameSingular} | ||
onCancel={() => closeDropdown()} | ||
onRecordSelected={handleRecordSelected} | ||
objectNameSingular={objectNameSingular} | ||
recordPickerInstanceId={dropdownId} | ||
/> | ||
} | ||
dropdownHotkeyScope={{ scope: dropdownId }} | ||
<Dropdown | ||
dropdownId={dropdownId} | ||
dropdownPlacement="bottom-start" | ||
clickableComponentWidth={'100%'} | ||
onClose={handleCloseRelationPickerDropdown} | ||
onOpen={handleOpenDropdown} | ||
clickableComponent={ | ||
<StyledFormSelectContainer | ||
hasRightElement={isDefined(VariablePicker) && !disabled} | ||
preventSetHotkeyScope={true} | ||
> | ||
<FormSingleRecordFieldChip | ||
draftValue={draftValue} | ||
selectedRecord={selectedRecord} | ||
objectNameSingular={objectNameSingular} | ||
onRemove={handleUnlinkVariable} | ||
/> | ||
</DropdownScope> | ||
)} | ||
</StyledFormSelectContainer> | ||
<StyledIconButton> | ||
<IconChevronDown | ||
size={theme.icon.size.md} | ||
color={theme.font.color.light} | ||
/> | ||
</StyledIconButton> | ||
</StyledFormSelectContainer> | ||
} | ||
dropdownComponents={ | ||
<SingleRecordPicker | ||
componentInstanceId={dropdownId} | ||
EmptyIcon={IconForbid} | ||
emptyLabel={'No ' + objectNameSingular} | ||
onCancel={() => closeDropdown()} | ||
onRecordSelected={handleRecordSelected} | ||
objectNameSingular={objectNameSingular} | ||
recordPickerInstanceId={dropdownId} | ||
/> | ||
} | ||
dropdownHotkeyScope={{ scope: dropdownId }} | ||
/> | ||
{isDefined(VariablePicker) && !disabled && ( | ||
<VariablePicker | ||
inputId={variablesDropdownId} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,19 +25,24 @@ import { isDefined } from 'twenty-shared/utils'; | |
import { useIsMobile } from 'twenty-ui/utilities'; | ||
import { useDropdown } from '../hooks/useDropdown'; | ||
|
||
type Width = `${string}px` | `${number}%` | 'auto' | number; | ||
const StyledDropdownFallbackAnchor = styled.div` | ||
left: 0; | ||
position: fixed; | ||
top: 0; | ||
`; | ||
|
||
const StyledClickableComponent = styled.div` | ||
const StyledClickableComponent = styled.div<{ | ||
width?: Width; | ||
}>` | ||
height: fit-content; | ||
width: ${({ width }) => width ?? 'auto'}; | ||
`; | ||
|
||
export type DropdownProps = { | ||
className?: string; | ||
clickableComponent?: ReactNode; | ||
clickableComponentWidth?: Width; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I know you took the time to find the best solution to this problem, but I wonder if we didn't find an API design flaw in the Dropdown component. We can provide a |
||
dropdownComponents: ReactNode; | ||
hotkey?: { | ||
key: Keys; | ||
|
@@ -46,7 +51,7 @@ export type DropdownProps = { | |
dropdownHotkeyScope: HotkeyScope; | ||
dropdownId: string; | ||
dropdownPlacement?: Placement; | ||
dropdownWidth?: `${string}px` | `${number}%` | 'auto' | number; | ||
dropdownWidth?: Width; | ||
dropdownOffset?: DropdownOffset; | ||
dropdownStrategy?: 'fixed' | 'absolute'; | ||
onClickOutside?: () => void; | ||
|
@@ -70,6 +75,7 @@ export const Dropdown = ({ | |
onClose, | ||
onOpen, | ||
avoidPortal, | ||
clickableComponentWidth = 'auto', | ||
}: DropdownProps) => { | ||
const { isDropdownOpen, toggleDropdown } = useDropdown(dropdownId); | ||
|
||
|
@@ -159,6 +165,7 @@ export const Dropdown = ({ | |
aria-expanded={isDropdownOpen} | ||
aria-haspopup={true} | ||
role="button" | ||
width={clickableComponentWidth} | ||
> | ||
{clickableComponent} | ||
</StyledClickableComponent> | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -25,10 +25,10 @@ import { BaseWorkflowActionSettings } from 'src/modules/workflow/workflow-execut | |
import { | ||
WorkflowAction, | ||
WorkflowActionType, | ||
WorkflowFormAction, | ||
} from 'src/modules/workflow/workflow-executor/workflow-actions/types/workflow-action.type'; | ||
import { WorkflowRunWorkspaceService } from 'src/modules/workflow/workflow-runner/workflow-run/workflow-run.workspace-service'; | ||
import { WorkflowRunnerWorkspaceService } from 'src/modules/workflow/workflow-runner/workspace-services/workflow-runner.workspace-service'; | ||
|
||
const TRIGGER_STEP_ID = 'trigger'; | ||
|
||
const BASE_STEP_DEFINITION: BaseWorkflowActionSettings = { | ||
|
@@ -287,16 +287,28 @@ export class WorkflowVersionStepWorkspaceService { | |
); | ||
} | ||
|
||
if (step.type !== WorkflowActionType.FORM) { | ||
throw new WorkflowVersionStepException( | ||
'Step is not a form', | ||
WorkflowVersionStepExceptionCode.INVALID, | ||
); | ||
} | ||
|
||
const enrichedResponse = await this.enrichFormStepResponse({ | ||
step, | ||
response, | ||
}); | ||
|
||
const newStepOutput: StepOutput = { | ||
id: stepId, | ||
output: { | ||
result: response, | ||
result: enrichedResponse, | ||
}, | ||
}; | ||
|
||
const updatedContext = { | ||
...workflowRun.context, | ||
[stepId]: response, | ||
[stepId]: enrichedResponse, | ||
}; | ||
|
||
await this.workflowRunWorkspaceService.saveWorkflowRunState({ | ||
|
@@ -547,4 +559,48 @@ export class WorkflowVersionStepWorkspaceService { | |
); | ||
} | ||
} | ||
|
||
private async enrichFormStepResponse({ | ||
step, | ||
response, | ||
}: { | ||
step: WorkflowFormAction; | ||
response: object; | ||
}) { | ||
const responseKeys = Object.keys(response); | ||
|
||
const enrichedResponses = await Promise.all( | ||
responseKeys.map(async (key) => { | ||
if (!isDefined(response[key])) { | ||
return { key, value: response[key] }; | ||
} | ||
|
||
const field = step.settings.input.find((field) => field.name === key); | ||
|
||
if ( | ||
field?.type === 'RECORD' && | ||
field?.settings?.objectName && | ||
isDefined(response[key]?.id) | ||
) { | ||
thomtrp marked this conversation as resolved.
Show resolved
Hide resolved
|
||
const repository = await this.twentyORMManager.getRepository( | ||
field.settings.objectName, | ||
); | ||
|
||
const record = await repository.findOne({ | ||
where: { id: response[key].id }, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. As discussed in private, an error is thrown here after having clicked on the option "No notes" because we send a string that's not a UUID and that throws here. |
||
}); | ||
|
||
return { key, value: record }; | ||
} else { | ||
return { key, value: response[key] }; | ||
} | ||
}), | ||
); | ||
|
||
return enrichedResponses.reduce((acc, { key, value }) => { | ||
acc[key] = value; | ||
|
||
return acc; | ||
}, {}); | ||
} | ||
} |
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.
Let's create stories for this component. We did a great job writing stories for most of the Form components, and this component shouldn't be an exception.