Fix/pci private registry fix restriction tapc xxx #14916
Conversation
ppprevost
requested review from
acavalloni,
MaximeBajeux,
sidlynx,
kqesar and
anooparveti
and removed request for
a team
| @@ -88,7 +89,7 @@ export const useDatagridColumn = () => { | |||
| ) : ( | |||
| <div>{capitalizeAndJoin(props.authorization)}</div> | |||
| ), | |||
| label: t('private_registry_cidr_authorization'), | |||
| label: ((<AuthorizationLabel />) as unknown) as string, | |||
There was a problem hiding this comment.
Could you add a comment to explain why you need this
There was a problem hiding this comment.
Hacky but do not have choice to implement a Popover next to label
There was a problem hiding this comment.
I understand, maybe you could add a typescript comment above this line to explain
| }); | ||
| const { t } = useTranslation(['ip-restrictions']); | ||
|
|
||
| const uniqueValue = generateUniqueString(dataCIDR.rows); |
There was a problem hiding this comment.
Should be in a useMemo
| columnFilters={columnFilters} | ||
| key={dataCIDR.totalRows} | ||
| key={uniqueValue} |
There was a problem hiding this comment.
I don't understand why is this required
There was a problem hiding this comment.
Yes agree with you for me no Need of this key here
There was a problem hiding this comment.
I need this key to refresh context, the behavior in other app is to manage pagination inside
if i do not use this key i need to refactor again inside the context to use.
There was a problem hiding this comment.
Do you know which variable isn't refreshed ? The context and the components using it should be updated, there might be a useMemo or useCallback not correctly configured
| const { t } = useTranslation(['ip-restrictions']); | ||
| return ( | ||
| <Controller | ||
| name="authorization" | ||
| control={control} | ||
| render={({ field: { onChange, value } }) => ( | ||
| render={({ field: { onChange, value }, fieldState: { isDirty } }) => ( |
There was a problem hiding this comment.
What does isDirty mean?
There was a problem hiding this comment.
isDirty means value touched and fill
There was a problem hiding this comment.
Sorry but I can't make the relation ...
isDirty sounds like code smell to me ;)
| * @returns {string} - A unique Base64-encoded string representing the filtered objects. | ||
| * @template T - The generic type of the objects in the array. | ||
| */ | ||
| export function generateUniqueString<T>(array: T[]): string { |
There was a problem hiding this comment.
I can't see the purpose of using a custom function to generate a unique key in base64 that is never decoded.
If the goal is to only generate a unique string, why not just using an uuid ?
There was a problem hiding this comment.
nope the main goal is to refresh the id of the key when data change
| .sort() | ||
| .join('|'); | ||
|
|
||
| return btoa(uniqueString); // Encode en base64 pour une chaîne compacte |
| @@ -88,7 +89,7 @@ export const useDatagridColumn = () => { | |||
| ) : ( | |||
| <div>{capitalizeAndJoin(props.authorization)}</div> | |||
| ), | |||
| label: t('private_registry_cidr_authorization'), | |||
| label: ((<AuthorizationLabel />) as unknown) as string, | |||
There was a problem hiding this comment.
I can't understand clearly why you use here a React Component and cast it as a string?
There was a problem hiding this comment.
I think the DataGrid only accepts string as type but in reality can accept ReactNode so this is a tweak because of wrong typing
There was a problem hiding this comment.
It's better change the type of DatagridColumn.label in packages/manager-react-components/src/components/datagrid/datagrid.component.tsxfor the next I think
|
@ppprevost |
ref: TAPC-2591 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2589 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2589 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2585 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2583 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2581 Signed-off-by: Pierre-Philippe <[email protected]>
ref: TAPC-2591 Signed-off-by: Pierre-Philippe <[email protected]>
ppprevost
force-pushed
the
fix/pci-private-registry_fix-restriction_tapc-xxx
branch
from
7f10638 to
fdacddd
Compare
ref: TAPC-2578 Signed-off-by: Pierre-Philippe <[email protected]>
ref:TAPC-2591
|
| <Trans ns="ip-restrictions"> | ||
| <OsdsText | ||
| color={ODS_THEME_COLOR_INTENT.text} | ||
| level={ODS_TEXT_LEVEL.body} | ||
| > | ||
| {t('private_registry_cidr_help_authorized_component')} | ||
| </OsdsText> | ||
| <br /> | ||
| <OsdsText | ||
| color={ODS_THEME_COLOR_INTENT.text} | ||
| level={ODS_TEXT_LEVEL.body} | ||
| > | ||
| {t('private_registry_cidr_help_authorized_component_part2')} | ||
| </OsdsText> | ||
| <br /> | ||
| <OsdsText | ||
| color={ODS_THEME_COLOR_INTENT.text} | ||
| level={ODS_TEXT_LEVEL.body} | ||
| > | ||
| {t('private_registry_cidr_help_authorized_component_part3')} | ||
| </OsdsText> | ||
| </Trans> |
There was a problem hiding this comment.
If we just want add spacing between element. Maybe use container with gap (flex flex-col gap-2) ?
Allow to have better control ?
| dataGrid={{ | ||
| ...dataGrid, | ||
| }} |
There was a problem hiding this comment.
Maybe better copy value on provider and not on the usage ?
We can have case we just want send the reference of datagrid ?
| }; | ||
|
|
||
| const DatagridProvider = <TData extends DatagridAction[]>({ | ||
| children, | ||
| data, | ||
| dataGrid, | ||
| columnFilters, | ||
| totalRows, |
There was a problem hiding this comment.
It's not just const totalRows = useMemo(() => data.length, [data]) ?
Description
Related