Remove extra buttons to create VM or MigrationPolicy #1237
Conversation
hstastna
changed the title
hstastna
force-pushed
the
Remove_extra_button_to_create_resource
branch
5 times, most recently
from
9d82bca
to
13a34f8
Compare
hstastna
changed the title
|
@avivtur @metalice @pcbailey @upalatucci @vojtechszocs please review |
|
|
||
| type VirtualMachinesCreateButtonProps = { | ||
| namespace: string; | ||
| buttonText?: string; |
There was a problem hiding this comment.
I think it's best to remove this prop and label the button to be simply "Create", the context should be clear as we are on the VM list page, wdyt?
There was a problem hiding this comment.
For such decisions, we have the UX folks. I just wanted to keep the original naming of the buttons in specific situations (such as presence of some VMs vs. no VMs at all) in this PR.
We have many inconsistencies across the whole UI like this for example, and it is not so straightforward to solve them, so I totally understand this wasn't improved yet by the UX. Actually, Yifat is overwhelmed by many questions from me, regarding such issues in our UI, and she's working on them one by one, it takes time...
...tionpolicies/list/components/MigrationPoliciesCreateButton/MigrationPoliciesCreateButton.tsx
Show resolved
Hide resolved
hstastna
force-pushed
the
Remove_extra_button_to_create_resource
branch
from
13a34f8
to
5234c3c
Compare
|
/retest |
...tionpolicies/list/components/MigrationPoliciesCreateButton/MigrationPoliciesCreateButton.tsx
Outdated
Show resolved
Hide resolved
| href={href} | ||
| target="_blank" | ||
| rel="noopener noreferrer" | ||
| data-test-id={dataTestID} | ||
| {...(stopPropagation ? { onClick: (e) => e.stopPropagation() } : {})} | ||
| > | ||
| {children || text} | ||
| {children || text} <ExternalLinkAltIcon /> |
There was a problem hiding this comment.
This is a generic file we are using across our app does everything looks the same after the change?
co-external-link === ExternalLinkAltIcon , from an appearance perspective?
There was a problem hiding this comment.
With this change, the icon has finally the correct size, and everywhere. It was too small and everywhere, according to the designs we have.
hstastna
force-pushed
the
Remove_extra_button_to_create_resource
branch
2 times, most recently
from
3a7f978
to
feff163
Compare
hstastna
force-pushed
the
Remove_extra_button_to_create_resource
branch
2 times, most recently
from
6aa29dd
to
dbaa135
Compare
|
@avivtur @metalice @pcbailey @upalatucci @vojtechszocs please review |
Don't display the unnecessary buttons to create VirtualMachine or MigrationPolicy in the empty state pages (no resources present), as there is the button to create the appropriate resource present also in the center of the page, which is the default for Patternfly empty state. Make this change according to the UX suggestions.
hstastna
force-pushed
the
Remove_extra_button_to_create_resource
branch
from
dbaa135
to
a2409c2
Compare
|
/lgtm |
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: hstastna, vojtechszocs 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 |
📝 Description
This is a followup PR for the comment:
#1234 (comment)
Don't display the unnecessary buttons to create VirtualMachine or MigrationPolicy in the empty state pages (no resources present), as there is the button to create the appropriate resource present also in the center of the page, which is the default for Patternfly empty state. Make this change according to the UX suggestions.
Create new
MigrationPoliciesCreateButtoncomponent to be used where needed.Update
MigrationPoliciesEmptyStatecomponent: use preferredExternalLinkcomponent instead ofain there.Also remove unnecessary pagination for VM's empty state page, as it doesn't make sense for this case.
🎥 Screenshots
Before:
MigrationPolicies empty state - 2 buttons to create the resource present in the page (and no options to create a policy from in the middle button):
VirtualMachines empty state - 2 buttons to create the resource present in the page (and no options to create a VM from in the middle button):
After:
MigrationPolicies empty state - only 1 button to create the resource present in the page (together w all the options to create a policy from):
VirtualMachines empty state - only 1 button to create the resource present in the page (plus options to create a VM from present in the middle button):
TODO: