Bug 2132721: Add missing texts to Delete dialogs #926
Conversation
|
@hstastna: This pull request references Bugzilla bug 2132721, which is invalid:
Comment In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
/bugzilla refresh |
|
@hstastna: This pull request references Bugzilla bug 2132721, which is valid. The bug has been moved to the POST state. The bug has been updated to refer to the pull request using the external bug tracker. 3 validation(s) were run on this bug
Requesting review from QA contact: In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
|
@openshift-ci[bot]: GitHub didn't allow me to request PR reviews from the following users: lkladnit. Note that only kubevirt-ui members and repo collaborators can review this PR, and authors cannot review their own PRs. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
hstastna
force-pushed
the
BZ_Delete_dialogs
branch
3 times, most recently
from
5a80a16
to
86d5906
Compare
|
@avivtur @metalice @pcbailey @yzamir @vojtechszocs please review |
| </Alert> | ||
| </Alert> | ||
| </Trans> | ||
| <Popover |
There was a problem hiding this comment.
Is there a benefit to using a ref for the Popover instead of wrapping the button with it?
There was a problem hiding this comment.
Yes. Wrapping the button with it - as it was originally here, didn't work to display the popover correctly.
It's because you simply cannot put there anything as a child for the PF Popover component - causes improper functionality or not displaying the child at all (as it was in our case).
There was a problem hiding this comment.
@hstastna I think we can do it without ref. If we need a ref it feels like the usage of the popover is missing something.. please check
There was a problem hiding this comment.
Yes. Popover component seems to not working 100% correctly. This seems to be related to Patternfly and changes of their components we use. And any other component around could influence that the original solution stopped working, not only Popover..
IMO fixing this can take some time, so I'd suggest to better go for the actual proposed and working solution, until we find some better way. IMO using ref is nothing bad here, just a different way that could be used.
src/views/templates/details/tabs/disks/components/DiskRowActions.tsx
Outdated
Show resolved
Hide resolved
src/views/virtualmachines/details/tabs/disk/modal/DeleteDiskModal.tsx
Outdated
Show resolved
Hide resolved
src/views/catalog/wizard/tabs/disks/components/DiskRowActions.tsx
Outdated
Show resolved
Hide resolved
hstastna
force-pushed
the
BZ_Delete_dialogs
branch
2 times, most recently
from
68b28e4
to
9faf95b
Compare
hstastna
requested review from
metalice and
pcbailey
and removed request for
upalatucci,
avivtur and
metalice
src/utils/components/ConfirmActionMessage/ConfirmActionMessage.tsx
Outdated
Show resolved
Hide resolved
hstastna
force-pushed
the
BZ_Delete_dialogs
branch
2 times, most recently
from
47bec05
to
2c0c91c
Compare
src/utils/components/ConfirmActionMessage/ConfirmActionMessage.tsx
Outdated
Show resolved
Hide resolved
hstastna
force-pushed
the
BZ_Delete_dialogs
branch
from
2c0c91c
to
349d84b
Compare
Add missing "?" or some more text in the following dialogs: - Delete VM - Detach VM disk - Delete VM NIC - Delete VM template - Delete VM template disk - Delete VM Template NIC - Delete VirtualMachineSnapshot - Delete MigrationPolicy - Edit Boot source reference - Delete NIC dialog while customizing VM - Delete VM Snapshot from the Snapshots card in the Overview tab Also delete unnecessary words in some of the related kebab menus for consistency reasons. Fixes https://bugzilla.redhat.com/show_bug.cgi?id=2132721
hstastna
force-pushed
the
BZ_Delete_dialogs
branch
from
349d84b
to
20c06bd
Compare
|
/lgtm |
1 similar comment
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hstastna, pcbailey 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 |
|
@hstastna: All pull requests linked via external trackers have merged: Bugzilla bug 2132721 has been moved to the MODIFIED state. In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes/test-infra repository. |
📝 Description
Fixes:
https://bugzilla.redhat.com/show_bug.cgi?id=2132721
Add missing "?" or some more text to the following dialogs:
Also delete unnecessary words in some of the related kebab menus for consistency reasons.
🎥 Screenshots (not all the changes included)
Before:
Delete VirtualMachine dialog:
Delete template NIC dialog:
VM Snapshot kebab menu:
Delete MigrationPolicy:
Edit Boot source reference:
After: