Skip to content
This repository has been archived by the owner on Jan 12, 2023. It is now read-only.

Select VMs table: add filter by VM power state #811

Merged
merged 3 commits into from Oct 20, 2021

Conversation

mturley
Copy link
Collaborator

@mturley mturley commented Oct 20, 2021

@seanforyou23 we missed this when implementing https://bugzilla.redhat.com/show_bug.cgi?id=1953253.

The second commit fixes something weird I was seeing with tooltips: when the rows get rearranged by the filter, the VMNameWithPowerState components weren't getting remounted, so the event handlers weren't being rebound on the Tooltips even though new DOM nodes were being created. I found that if a VM row didn't end up in the same position in the table, its tooltip stopped working. Putting a key in there that is based on the row index and VM name forces it to remount and create new tooltips, which fixes it.

Screen Shot 2021-10-20 at 3 25 45 PM

Screen Shot 2021-10-20 at 3 25 51 PM

Screen Shot 2021-10-20 at 3 25 56 PM

@mturley mturley requested a review from a team October 20, 2021 20:24
@github-actions
Copy link

Bug is not valid because it is in ON_QA and should be one of NEW, ASSIGNED, or POST

@mturley mturley changed the title Bug 1953253: Select VMs table: add filter by VM power state Select VMs table: add filter by VM power state Oct 20, 2021
@konveyor-preview-bot
Copy link

🚀 Deployed Preview: http://konveyor-forklift-ui-pr-811-preview.surge.sh

Compare with current main branch: http://konveyor-forklift-ui-preview.surge.sh

@sonarcloud
Copy link

sonarcloud bot commented Oct 20, 2021

SonarCloud Quality Gate failed.    Quality Gate failed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 1 Code Smell

No Coverage information No Coverage information
5.5% 5.5% Duplication

Copy link
Contributor

@seanforyou23 seanforyou23 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

<VMNameWithPowerState
vm={vm}
sourceProvider={sourceProvider}
key={`row${rows.length}-${vm.name}`} // Ensure it always re-mounts when table rows change so tooltip state doesn't get messed up
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 nice work identifying the cause of that issue, it can be a tricky bug to hunt down

@mturley mturley merged commit 4025c10 into kubev2v:main Oct 20, 2021
@mturley mturley deleted the bz1953253-power-state-filter branch October 20, 2021 20:52
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants