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

scroll fields from added hetero-list entry into viewport #9488

Merged
merged 3 commits into from
Jul 28, 2024

Conversation

mawinter69
Copy link
Contributor

@mawinter69 mawinter69 commented Jul 20, 2024

The current logic in ensureVisible seems to have no effect, so it is changed in the following way:

  1. When the element is bigger then the current viewport height, scroll so that the element is at the top.
  2. When the elements is smaller than the viewport height and the bottom is outside, scroll so that the bottom is at the lower end of the viewport.
  3. Otherwise no scrolling is needed as the element is already completely inside the viewport.

This also replaces usage of YUI with plain javascript in the ensureVisible method

Bildschirmaufnahme.2024-07-20.141757.mp4
Bildschirmaufnahme.2024-07-20.141607.mp4
Bildschirmaufnahme.2024-07-20.141649.mp4

Testing done

Manual testing

Proposed changelog entries

  • Scroll fields from added hetero-list entry into viewport.

Proposed upgrade guidelines

N/A

Submitter checklist

Desired reviewers

@mention

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

Maintainer checklist

replace usage of YUI with plain javascript in the ensureVisible method

The current logic seems to have no effect, so it is changed in the
following
When the element is bigger then the current viewport height, scroll so
that the element is at the top.
If the elements is smaller than the viewport height and the bottom is
outside scoll so that the bottom is at the lower end of the viewport.
Otherwise no scrolling is needed the element is already completely
inside the viewport.
@timja timja added needs-ath-build Needs to run through the full acceptance-test-harness suite rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise labels Jul 21, 2024
@timja timja requested a review from a team July 21, 2024 15:52
@timja
Copy link
Member

timja commented Jul 21, 2024

@mawinter69 you able to run an ATH build please? It can be sensitive to this sort of thing

@mawinter69
Copy link
Contributor Author

I've run ath here: jenkinsci/acceptance-test-harness#1628 without problems

@timja timja added ath-successful This PR has successfully passed the full acceptance-test-harness suite and removed needs-ath-build Needs to run through the full acceptance-test-harness suite labels Jul 23, 2024
@timja timja requested review from zbynek and a team July 23, 2024 06:50
@timja
Copy link
Member

timja commented Jul 25, 2024

/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 Jul 25, 2024
@MarkEWaite MarkEWaite merged commit 90fb0d6 into jenkinsci:master Jul 28, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ath-successful This PR has successfully passed the full acceptance-test-harness suite ready-for-merge The PR is ready to go, and it will be merged soon if there is no negative feedback rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted web-ui The PR includes WebUI changes which may need special expertise
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants