feat(advanced-search): adding select value modal #6026
Conversation
| margin-right: 4px; | ||
| `; | ||
|
|
||
| export const SelectContainerModal = ({ onCloseModal, defaultValues, onOkOverride, titleOverride }: Props) => { |
There was a problem hiding this comment.
nit: would you mind changing this or the file name so they're consistent?
|
|
||
| // Renders a search result in the select dropdown. | ||
| const renderSearchResult = (entity: Container) => { | ||
| const displayName = entity.properties?.name || entity.properties?.qualifiedName || entity.urn; |
There was a problem hiding this comment.
could you use entityRegistry.getDisplayName()?
| const filteredContainers = | ||
| containerSearchResults?.filter((entity) => entity.urn === newUrn).map((entity) => entity) || []; | ||
|
|
||
| if (filteredContainers.length) { | ||
| const container = filteredContainers[0] as Container; |
There was a problem hiding this comment.
nit: I don't think you need this map function. also you could use containerSearchResults?.find(entity => entity.urn === newUrn) and then you'd have the container right then if it exists!
| label: defaultValue.entity ? renderDropdownResult(defaultValue.entity) : defaultValue.urn, | ||
| value: { | ||
| ownerUrn: defaultValue.urn, | ||
| ownerEntityType: EntityType.CorpUser, |
There was a problem hiding this comment.
could this be a CorpGroup ever?
| const [selectedOwners, setSelectedOwners] = useState<SelectedOwner[]>( | ||
| defaultValuesToSelectedOwners(defaultValues || []), | ||
| ); | ||
| console.log({ selectedOwners }); |
There was a problem hiding this comment.
can remove this bad boy
| console.log({ | ||
| deselecting: 'deselecting', | ||
| selectedValue, | ||
| selectedOwners, | ||
| }); |
There was a problem hiding this comment.
also can remove this one
| ); | ||
| } | ||
|
|
||
| if (filterField === 'container') { |
There was a problem hiding this comment.
could be done in a followup, but it might be nice to have an enum or constants for these different types of filterFields instead of just strings here
There was a problem hiding this comment.
ah i see you commented the same thing
There was a problem hiding this comment.
will do in the final pr 👍
| <Button onClick={onCloseModal} type="text"> | ||
| Cancel | ||
| </Button> | ||
| <Button disabled={stagedValue.length === 0} onClick={() => onOkOverride?.(stagedValue)}> |
There was a problem hiding this comment.
onOkOverride makes it seem like there's a default onOk method going on here but there's not. How would you feel about just calling the prop onOk and making it required? I think you can do this with ContainerSelectModal, SelectPlatformModal, and EditTextModal below as well
| }: EditTagsModalProps) { | ||
| console.log({ defaultValues }); |
| if (entity?.type === EntityType.Tag) { | ||
| return ( | ||
| <TagLabel | ||
| name={(entity as Tag).properties?.name || (entity as Tag).name} |
There was a problem hiding this comment.
can you use entityRegistry.getDisplayName here?
...ub-web-react/src/app/entity/shared/components/styled/search/action/GlossaryTermsDropdown.tsx
Show resolved
Hide resolved
...eb-react/src/app/entity/shared/containers/profile/sidebar/Container/ContainerSelectModal.tsx
Outdated
Show resolved
Hide resolved
| ); | ||
| } | ||
|
|
||
| if (filterField === 'container') { |
There was a problem hiding this comment.
ah i see you commented the same thing
...hub-web-react/src/app/entity/shared/containers/profile/sidebar/Ownership/EditOwnersModal.tsx
Outdated
Show resolved
Hide resolved
This is a helper component that advanced search filters will use to select values.
Checklist