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

[JENKINS-72679] Allow button clicks after an administrative monitor popup is closed #8941

Merged
merged 1 commit into from
Feb 9, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Feb 7, 2024

The div of the admin monitor pop-up is set to opacity 0 but keeps z-index 1000 when closed. This makes the buttons, that are behind the popup when shown unusable.
This change uses an animation that ensures the z-index is 0 after hiding the popup and buttons are still usable

Before:

Broken.mp4

After:

fixed.mp4

Testing done

Manually testing

Proposed changelog entries

  • Allow button clicks after closing an administrative monitor popup.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

N/A

Before the changes are marked as ready-for-merge:

Maintainer checklist

The div of the admin monitor pop-up is set to opacity 0 but keeps
z-index 1000 when closed. This makes the buttons, that are behind the
popup when shown unusable.
This change uses an animation that ensures the z-index is 0 after hiding
the popup and buttons are still usable
@mawinter69 mawinter69 changed the title avoid admin monitor popup makes buttons unusable [JENKINS-72679] avoid admin monitor popup makes buttons unusable Feb 7, 2024
@NotMyFault NotMyFault added the regression-fix Pull request that fixes a regression in one of the previous Jenkins releases label Feb 8, 2024
@NotMyFault NotMyFault requested a review from a team February 8, 2024 08:59
Copy link
Member

@timja timja left a comment

Choose a reason for hiding this comment

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

/label ready-for-merge


This PR is now ready for merge, after ~24 hours, we will merge it if there's no negative feedback.

Thanks!

@comment-ops-bot comment-ops-bot bot added the ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback label Feb 8, 2024
@NotMyFault NotMyFault merged commit a642354 into jenkinsci:master Feb 9, 2024
16 checks passed
@MarkEWaite MarkEWaite changed the title [JENKINS-72679] avoid admin monitor popup makes buttons unusable [JENKINS-72679] Allow button clicks after closing an administrative monitor popup. Feb 9, 2024
@MarkEWaite MarkEWaite changed the title [JENKINS-72679] Allow button clicks after closing an administrative monitor popup. [JENKINS-72679] Allow button clicks after closing an administrative monitor popup Feb 9, 2024
@MarkEWaite MarkEWaite changed the title [JENKINS-72679] Allow button clicks after closing an administrative monitor popup [JENKINS-72679] Allow button clicks after an administrative monitor popup is closed Feb 9, 2024
@daniel-beck
Copy link
Member

daniel-beck commented Feb 12, 2024

With this change, the hiding animation plays when the page is initially showing (with an empty popup, a fairly small square).

PR-8941-regression.mov

Enough to revert, or follow up separately?

@mawinter69
Copy link
Contributor Author

I'll provide a fix

mawinter69 added a commit to mawinter69/jenkins that referenced this pull request Feb 12, 2024
PR jenkinsci#8941 caused a regression that the hiding animation was played on
page load.
This change ensures that the hiding animation is only applied when the
widget was visible before
@daniel-beck
Copy link
Member

This change does not properly remove the admin monitor UI elements, still resulting in unintentional interactions with the admin monitors (discovered and recorded with the #8954 PR build, but it's also in here).

pr-8941-incomplete.mov

daniel-beck added a commit to daniel-beck/jenkins that referenced this pull request Feb 13, 2024
NotMyFault pushed a commit that referenced this pull request Feb 13, 2024
…actable UI elements (#8954)

* followup for #8941, don't animate on page load

PR #8941 caused a regression that the hiding animation was played on
page load.
This change ensures that the hiding animation is only applied when the
widget was visible before

* scale to 0
NotMyFault pushed a commit to NotMyFault/jenkins that referenced this pull request Feb 13, 2024
…kinsci#8941)

avoid admin monitor makes buttons unusable

The div of the admin monitor pop-up is set to opacity 0 but keeps
z-index 1000 when closed. This makes the buttons, that are behind the
popup when shown unusable.
This change uses an animation that ensures the z-index is 0 after hiding
the popup and buttons are still usable

(cherry picked from commit a642354)
NotMyFault pushed a commit to NotMyFault/jenkins that referenced this pull request Feb 13, 2024
…actable UI elements (jenkinsci#8954)

* followup for jenkinsci#8941, don't animate on page load

PR jenkinsci#8941 caused a regression that the hiding animation was played on
page load.
This change ensures that the hiding animation is only applied when the
widget was visible before

* scale to 0

(cherry picked from commit e5fd912)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback regression-fix Pull request that fixes a regression in one of the previous Jenkins releases
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants