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
kubevirt: add source and remove type from disks table #3562
kubevirt: add source and remove type from disks table #3562
Conversation
/retest |
@suomiy let me look at the order again. This could just be an artifact of the old Create VM Flow where we had an inline edit flow where source needed to be first because it would determine which fields followed when editing/creating a disk. |
@suomiy I think we could now align better with the console by putting the name first. We'll still want to keep the source first in the modal b/c it impacts other fields. |
e2214ba
to
c3ebcc7
Compare
fixed, name comes first now |
c3ebcc7
to
59627d5
Compare
return { | ||
wizardStorageData, | ||
// for sorting | ||
name: combinedDisk.getName(), | ||
source: source && source.getValue(), |
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.
maybe the safe accessing can move to getSource()
?
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.
fixed
59627d5
to
347e48a
Compare
/lgtm |
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: rawagner, suomiy 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 |
/retest Please review the full test history for this PR and help us cut down flakes. |
1 similar comment
/retest Please review the full test history for this PR and help us cut down flakes. |
ok, this means we'll want to ensure it's the same everywhere now right (Name first)? |
not sure what you mean by that? The names are first in all tables, but not in the modals. |
ok, we have in some of our designs (maybe just CD-ROMs right now) where the source is still first so maybe that's the only one we need to update which I can do. FYI @glekner |
changing according to our recent UI discussions
@glekner @matthewcarleton
btw, shouldn't we keep the name first?