-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Support Full Name as Record Text Identifier #11610
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?
Changes from 1 commit
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 | ||||
---|---|---|---|---|---|---|
|
@@ -5,6 +5,7 @@ import { recordIndexOpenRecordInState } from '@/object-record/record-index/state | |||||
import { ObjectRecord } from '@/object-record/types/ObjectRecord'; | ||||||
import { ViewOpenRecordInType } from '@/views/types/ViewOpenRecordInType'; | ||||||
import { useRecoilValue } from 'recoil'; | ||||||
import { FieldMetadataType } from 'twenty-shared/types'; | ||||||
import { | ||||||
AvatarChip, | ||||||
AvatarChipVariant, | ||||||
|
@@ -23,6 +24,7 @@ export type RecordChipProps = { | |||||
to?: string | undefined; | ||||||
size?: ChipSize; | ||||||
isLabelHidden?: boolean; | ||||||
field?: { type: string; name: string }; | ||||||
}; | ||||||
|
||||||
export const RecordChip = ({ | ||||||
|
@@ -35,6 +37,7 @@ export const RecordChip = ({ | |||||
size, | ||||||
forceDisableClick = false, | ||||||
isLabelHidden = false, | ||||||
field, | ||||||
}: RecordChipProps) => { | ||||||
const { recordChipData } = useRecordChipData({ | ||||||
objectNameSingular, | ||||||
|
@@ -70,12 +73,18 @@ export const RecordChip = ({ | |||||
}) | ||||||
: undefined; | ||||||
|
||||||
let name = recordChipData.name; | ||||||
|
||||||
if (field?.type === FieldMetadataType.FULL_NAME) { | ||||||
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. see my comment above: this should be handled in useRecordChipData |
||||||
name = `${record[field.name]?.firstName || ''} ${record[field.name]?.lastName || ''}`; | ||||||
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. style: Consider adding .trim() to remove extra space when either firstName or lastName is empty
Suggested change
|
||||||
} | ||||||
|
||||||
return ( | ||||||
<LinkAvatarChip | ||||||
size={size} | ||||||
maxWidth={maxWidth} | ||||||
placeholderColorSeed={record.id} | ||||||
name={recordChipData.name} | ||||||
name={name} | ||||||
isLabelHidden={isLabelHidden} | ||||||
avatarType={recordChipData.avatarType} | ||||||
avatarUrl={recordChipData.avatarUrl ?? ''} | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,7 @@ export const ChipFieldDisplay = () => { | |
objectNameSingular, | ||
labelIdentifierLink, | ||
isLabelIdentifierCompact, | ||
fieldDefinition, | ||
} = useChipFieldDisplay(); | ||
|
||
if (!isDefined(recordValue)) { | ||
|
@@ -22,6 +23,10 @@ export const ChipFieldDisplay = () => { | |
size={ChipSize.Small} | ||
to={labelIdentifierLink} | ||
isLabelHidden={isLabelIdentifierCompact} | ||
field={{ | ||
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. see comment above |
||
type: fieldDefinition.type, | ||
name: fieldDefinition.metadata.fieldName, | ||
}} | ||
/> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -13,7 +13,8 @@ import { useSnackBar } from '@/ui/feedback/snack-bar-manager/hooks/useSnackBar'; | |
import { Select } from '@/ui/input/components/Select'; | ||
import { zodResolver } from '@hookform/resolvers/zod'; | ||
import { t } from '@lingui/core/macro'; | ||
import { IconCircleOff, useIcons } from 'twenty-ui/display'; | ||
import { useNavigate } from 'react-router-dom'; | ||
import { IconCircleOff, IconPlus, useIcons } from 'twenty-ui/display'; | ||
import { SelectOption } from 'twenty-ui/input'; | ||
|
||
export const settingsDataModelObjectIdentifiersFormSchema = | ||
|
@@ -100,6 +101,9 @@ export const SettingsDataModelObjectIdentifiersForm = ({ | |
label: 'None', | ||
value: null, | ||
}; | ||
|
||
const navigate = useNavigate(); | ||
|
||
return ( | ||
<StyledContainer> | ||
{[ | ||
|
@@ -124,12 +128,23 @@ export const SettingsDataModelObjectIdentifiersForm = ({ | |
render={({ field: { onChange, value } }) => ( | ||
<Select | ||
label={label} | ||
disabled={!objectMetadataItem.isCustom || !options.length} | ||
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 don't understand this change either 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. In issue, there's mentioned in Desired Behavior section: Implement this design when only one field is available as text identifier. (Keep the field clickable, do not disable it) |
||
fullWidth | ||
dropdownId={`${fieldName}-select`} | ||
emptyOption={emptyOption} | ||
options={options} | ||
value={value} | ||
withSearchInput={label === t`Record label`} | ||
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 don't understand this change 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. In Issue, required change is only in 'Record Label' dropdown, Not in 'Record Image' dropdown. |
||
callToActionButton={ | ||
label === t`Record label` | ||
? { | ||
text: 'Create Text Field', | ||
Icon: IconPlus, | ||
onClick: () => { | ||
navigate('./new-field/select'); | ||
}, | ||
} | ||
: undefined | ||
} | ||
onChange={(value) => { | ||
onChange(value); | ||
formConfig.handleSubmit(handleSave)(); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -47,7 +47,6 @@ const reservedKeywords = [ | |
'links', | ||
'currency', | ||
'currencies', | ||
'fullName', | ||
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. why? 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. Without removing it, Its not allowing to create field named 'Full Name' (As I created in demo clip above). It throws error: The name "fullName" is not available |
||
'fullNames', | ||
Comment on lines
49
to
50
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. style: Consider whether 'fullNames' should also be removed for consistency since 'fullName' is now allowed |
||
'address', | ||
'addresses', | ||
|
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 should not pass the field here, and modify useRecordChipData instead!