-
Notifications
You must be signed in to change notification settings - Fork 28
Draft: MTV-2270: Make the "Cutover" button context aware #1537
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?
Conversation
Signed-off-by: Joachim Schuler <[email protected]>
Signed-off-by: Joachim Schuler <[email protected]>
Signed-off-by: Joachim Schuler <[email protected]>
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #1537 +/- ##
==========================================
- Coverage 36.81% 36.20% -0.61%
==========================================
Files 158 157 -1
Lines 2548 2596 +48
Branches 599 632 +33
==========================================
+ Hits 938 940 +2
- Misses 1428 1470 +42
- Partials 182 186 +4 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Signed-off-by: Joachim Schuler <[email protected]>
<Tooltip content={cutoverTime}> | ||
<Button | ||
variant="secondary" | ||
icon={<CalendarIcon />} | ||
onClick={onClickPlanCutoverMigration} | ||
> | ||
{t('Edit cutover')} | ||
</Button> | ||
</Tooltip> | ||
) : ( | ||
<Button variant="primary" icon={<CalendarIcon />} onClick={onClickPlanCutoverMigration}> | ||
{t('Schedule cutover')} | ||
</Button> |
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.
this looks very similar to the block you made in MigrationTypeCell.tsx file,
Please extract this component and use it in both places
{cutoverSet ? ( | ||
<ForkliftTrans> | ||
<p> | ||
Edit the cutover for migration <strong className="co-break-word">{name}</strong>? | ||
</p> | ||
</ForkliftTrans> | ||
) : ( | ||
<ForkliftTrans> | ||
<p> | ||
Schedule the cutover for migration <strong className="co-break-word">{name}</strong>? | ||
</p> | ||
</ForkliftTrans> | ||
)} |
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.
This can be simply:
<ForkliftTrans><p>{cutoverSet ? "Edit" : "Schedule"} the cutover for migration <strong className="co-break-word">{name}</strong>? </p></ForkliftTrans>
@@ -139,7 +137,7 @@ export const PlanCutoverMigrationModal: React.FC<PlanCutoverMigrationModalProps> | |||
isLoading={isLoading} | |||
isDisabled={!isDateValid || !isTimeValid} | |||
> | |||
{t('Set cutover')} | |||
{cutoverSet ? t('Edit cutover') : t('Schedule cutover')} |
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.
this and row 152 reapets itself, and I also see it PlanActionDropdownItems.tsx file, consider extracting it to a const or a util
@@ -45,8 +45,7 @@ export const PlanStatusCell: React.FC<CellProps> = ({ data }) => { | |||
const [lastMigration] = usePlanMigration(plan); | |||
const [isButtonEnabled, setIsButtonEnabled] = useState(true); | |||
|
|||
const isWarmAndExecuting = plan.spec?.warm && isPlanExecuting(plan); | |||
const isWaitingForCutover = isWarmAndExecuting && !isPlanArchived(plan); | |||
const isWarmAndExecuting = plan.spec?.warm && isPlanExecuting(plan) && !isPlanArchived(plan); |
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.
This part also repeat itself a few times, consider extracting it to a const/util func
|
||
const onClickPlanCutoverMigration = () => { | ||
showModal(<PlanCutoverMigrationModal resource={plan} />); | ||
}; | ||
|
||
const cutoverSet = Boolean(lastMigration?.spec?.cutover); |
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.
This part also repeat itself a few times, consider extracting it to a const/util func
|
||
const onClickPlanCutoverMigration = () => { | ||
showModal(<PlanCutoverMigrationModal resource={plan} />); | ||
}; | ||
|
||
const cutoverSet = Boolean(lastMigration?.spec?.cutover); | ||
const cutoverTime = DateTime.fromISO(lastMigration?.spec?.cutover).toLocaleString( |
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.
This part also repeat itself a few times, consider extracting it to a const/util func
@@ -19,7 +23,8 @@ import { | |||
V1beta1StorageMap, | |||
} from '@kubev2v/types'; | |||
import { useK8sWatchResource } from '@openshift-console/dynamic-plugin-sdk'; | |||
import { Level, List, ListItem, PageSection } from '@patternfly/react-core'; | |||
import { Button, Level, List, ListItem, PageSection, Tooltip } from '@patternfly/react-core'; | |||
import CalendarIcon from '@patternfly/react-icons/dist/esm/icons/calendar-alt-icon'; |
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.
please change this import to be import {CalendarIcon} from '@patternfly/react-icons' and not the dist directory
|
This PR is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 15 days. |
📝 Links
https://issues.redhat.com/browse/MTV-2270
📝 Description
In plans list, change "Cutover" button within the Migration type column to say either:
These text changes also extend to the actions menu item
The "Edit cutover" button within the Migration type column will also gain a tooltip with the currently set cutover date.
Within plan details, add a context aware Cutover button next to the actions dropdown.
🎥 Demo
Screen.Recording.2025-03-20.at.2.36.07.PM.mov
📝 CC://
@sgratch