Modified wizard flow with 4.15 specific changes #1100
Conversation
|
|
|
|
|
|
GowthamShanmugam
force-pushed
the
RHSTOR-4922-part-2
branch
from
069c538 to
c552335
Compare
GowthamShanmugam
changed the title
| children?: React.ReactNode; | ||
| }; | ||
|
|
||
| type ReviewAndCreationStepProps = ReviewAndCreateStep & { |
There was a problem hiding this comment.
| type ReviewAndCreationStepProps = ReviewAndCreateStep & { | |
| type ReviewAndCreationGroupProps = ReviewAndCreateStep & { |
| </DescriptionListGroup> | ||
| ); | ||
|
|
||
| type ReviewAndCreateStep = { |
There was a problem hiding this comment.
nits:
| type ReviewAndCreateStep = { | |
| type ReviewAndCreateStepProps = { |
| title?: React.ReactNode; | ||
| description?: React.ReactNode; |
There was a problem hiding this comment.
why all props are optional ??
There was a problem hiding this comment.
Ah! title can't be optional
| className?: string; | ||
| children?: React.ReactNode; |
There was a problem hiding this comment.
if all props are optional, what would this FC do if we do not pass anything to it ??
There was a problem hiding this comment.
my bad, children has to be mandatory
| !!dataPolicy.placementControlInfo.every((drpc) => !!drpc.pvcSelector?.length); | ||
| const isPVCSelectorFound = (pvcSelectors: PVCSelectorType[]) => | ||
| !!pvcSelectors.length && | ||
| !!pvcSelectors.every((pvcSelector) => !!pvcSelector?.labels?.length); |
There was a problem hiding this comment.
| !!pvcSelectors.every((pvcSelector) => !!pvcSelector?.labels?.length); | |
| !!pvcSelectors.every((pvcSelector) => !!pvcSelector.labels?.length); |
| ? pvcSelectors.map((pvcSelector) => | ||
| !!pvcSelector ? [pvcSelector.placementName, pvcSelector.labels] : [] |
There was a problem hiding this comment.
if u r doing a map over pvcSelectors, then pvcSelector should be there, why do we need ternary condition inside a callback ??
| ? t('{{count}} placements', { count: selectorCount }) | ||
| : pvcSelectors[0].placementName; | ||
|
|
||
| const labels = React.useMemo(() => getLabels(pvcSelectors), [pvcSelectors]); |
There was a problem hiding this comment.
do we really need useMemo here ??
There was a problem hiding this comment.
const getLabels = (pvcSelectors: PVCSelectorType[]): string[] =>
pvcSelectors.reduce((acc, selectors) => [...acc, ...selectors.labels], []);
get label is creating an array, it may cause re-render
There was a problem hiding this comment.
yes but we do not want to keep reference of labels same right ? we r not passing it to any dependency list or anything...
There was a problem hiding this comment.
unless u r setting state in getLabels, it will not cause re-render...
There was a problem hiding this comment.
ok got it, i will keep this in mind.
| drPolicy.spec.schedulingInterval !== '0m' | ||
| ? REPLICATION_TYPE.ASYNC | ||
| : REPLICATION_TYPE.SYNC, |
There was a problem hiding this comment.
are we using this check somewhere else ?? If so, we can create a separate utility function for this... else it is okay...
| @@ -636,3 +636,6 @@ export const findDeploymentClusters = ( | |||
| return !!lastDeploymentClusterName ? [lastDeploymentClusterName] : []; | |||
| } | |||
| }; | |||
|
|
|||
| export const getDRPolicyStatus = (isValidated, t) => | |||
| isValidated ? t('Validated') : t('Not Validated'); | |||
There was a problem hiding this comment.
nits: should we ??
| isValidated ? t('Validated') : t('Not Validated'); | |
| isValidated ? t('Validated') : t('Not validated'); |
GowthamShanmugam
force-pushed
the
RHSTOR-4922-part-2
branch
from
c552335 to
576b2b9
Compare
Signed-off-by: Gowtham Shanmugasundaram <[email protected]>
GowthamShanmugam
force-pushed
the
RHSTOR-4922-part-2
branch
from
576b2b9 to
7a699fc
Compare
|
/approve |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: GowthamShanmugam, SanjalKatiyar The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
openshift-merge-bot
bot
merged commit 5646fc8
into
red-hat-storage:master
It has a few 4.15-related changes for already existing wizard steps.