Skip to content
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

ACM-13648 - distribution version comparison (AcmTable advanced search iteration 2) #3902

Conversation

Randy424
Copy link
Contributor

Regarding: https://issues.redhat.com/browse/ACM-13648

Now supports distribution version.
Before:
Screenshot 2024-09-25 at 12 49 57 PM

After:
Screenshot 2024-09-25 at 12 32 17 PM

@Randy424 Randy424 force-pushed the ACM-13648-distribution-ver-comparison branch from bf93571 to 1b02179 Compare September 26, 2024 05:13
@Randy424 Randy424 force-pushed the ACM-13648-distribution-ver-comparison branch 3 times, most recently from 428b955 to e938a3a Compare September 26, 2024 09:50
Comment on lines 218 to 227
onMouseEnter={() => {
const newIsHoveredArray = { ...isHovered }
newIsHoveredArray[index] = true
setIsHovered(newIsHoveredArray)
}}
onMouseLeave={() => {
const newIsHoveredArray = { ...isHovered }
newIsHoveredArray[index] = false
setIsHovered(newIsHoveredArray)
}}
Copy link
Contributor Author

@Randy424 Randy424 Sep 26, 2024

Choose a reason for hiding this comment

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

Is the best way to handle this (they are helping me render the tooltip onMouseHover) to use a window listener?

Sonar is describes my method here as using a non-native interactive element.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think there's a simpler solution. Will this accomplish what you need?

                      <Tooltip
                        content={t('Select a column name to choose an operator')}
                        trigger={!constraint.columnId ? 'mouseenter' : 'manual'}
                        position="bottom"
                      >

@Randy424
Copy link
Contributor Author

/hold

Still need to refactor this bit

@Randy424 Randy424 force-pushed the ACM-13648-distribution-ver-comparison branch 2 times, most recently from c9a53ad to 49a5f95 Compare September 26, 2024 20:25
Signed-off-by: Randy Bruno Piverger <rbrunopi@redhat.com>
@Randy424 Randy424 force-pushed the ACM-13648-distribution-ver-comparison branch from 49a5f95 to 218b077 Compare September 26, 2024 20:53
@Randy424
Copy link
Contributor Author

Randy424 commented Sep 26, 2024

Update: added change for nullish coalescing operator code smell.

Copy link

sonarcloud bot commented Sep 26, 2024

@KevinFCormier
Copy link
Contributor

/lgtm

Copy link

openshift-ci bot commented Sep 26, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: KevinFCormier, Randy424

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:
  • OWNERS [KevinFCormier,Randy424]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@Randy424
Copy link
Contributor Author

/unhold

@openshift-merge-bot openshift-merge-bot bot merged commit 3a88e8e into stolostron:main Sep 26, 2024
9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants