Fix keyboard navigation scrolling in dropdowns

[SalmanDeveloperz]

Fixes #26357

Testing done

Screenshots (UI changes only)

Before

Arrow key navigation selected items but didn't scroll, making off-screen items invisible.

dropdown.scrolling.defect.mp4

After

Dropdown scrolls smoothly to center the selected item when navigating with arrow keys.

fix.dropdown.scrolling.mp4

Proposed changelog entries

• Fix keyboard navigation scrolling in dropdowns

Proposed changelog category

/label bug

Proposed upgrade guidelines

N/A

Submitter checklist

• The issue, if it exists, is well-described.
• The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood (see examples). Fill in the Proposed upgrade guidelines section only if there are breaking changes or changes that may require extra steps from users during upgrade.
• There is automated testing or an explanation as to why this change has no tests.
• New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
• New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
• UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin. In particular, new or substantially changed JavaScript is not defined inline and does not call eval to ease future introduction of Content Security Policy (CSP) directives (see documentation).
• For dependency updates, there are links to external changelogs and, if possible, full differentials.
• For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@timja

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

Maintainer checklist

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, be a Bug or Improvement, and either the issue or pull request must be labeled as lts-candidate to be considered.

[timja]

Whilst this does fix the bug (which is the priority) it is scrolling too early and its a bit jumpy.

Could you investigate if the isInViewport function could be fixed instead? as I assume that was intended to only scroll when needed. (cc @mawinter69 as the author of that change)

the block: center does seem better. When it was anchored to the bottom which is what false does it was good when going down but when going back up it was a bit strange feeling.

[SalmanDeveloperz]

Good point. I tested with block: 'nearest' initially, which only scrolls when needed, but it didn't show section headers when wrapping from bottom to top (e.g. selecting "System" didn't show the "Actions" and "System Configuration" header above it).

The isInViewport function checks if the element is in the browser window, not the dropdown container. Items scrolled out of view in the dropdown still pass the check because they're technically in the window, so scrolling never triggers.

I can fix isInViewport to check visibility within the dropdown bounds instead. That would keep the "only scroll when needed" behavior while fixing the bug. Want me to go that route?

[timja]

I can fix isInViewport to check visibility within the dropdown bounds instead. That would keep the "only scroll when needed" behavior while fixing the bug. Want me to go that route?

Give it a go, would be better if it was less jarring, but if we can't get something nicer at least this works.

[SalmanDeveloperz]

Will update to check dropdown container visibility. Testing now !

[SalmanDeveloperz]

Could you investigate if the isInViewport function could be fixed instead? as I assume that was intended to only scroll when needed. (cc @mawinter69 as the author of that change)

Tried fixing isInViewport to check dropdown container visibility but ran into issues, the dropdown doesn't have a max-height in the DOM, so it sizes to fit all items and visibility checks always pass.

The "nearest" approach scrolls smoothly and only when needed.
Minor edge case: section headers don't always show when wrapping bottom to top, but navigation works fine.

Video attached.

dropdown.with.nearest.fix.mp4

Worth committing or should try something else?

[timja]

yeah lets go for nearest

[SalmanDeveloperz]

Done!

[timja]

Tested locally, this works a lot better.

Yes the headings don't show when you loop back around when scrolling but meh its fine.

Thanks!

If you're looking for another similar issue, while testing this fix I noticed (and reported) https://github.com/jenkinsci/jenkins/issues/26378

[SalmanDeveloperz]

Thanks for testing! Will check out the ticket.

[andreahlert]

Tested. LGTM.

[timja]

/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!

[welcome]

Congratulations on getting your very first Jenkins core pull request merged 🎉🥳

This is a fantastic achievement, and we're thrilled to have you as part of our community! Thank you for your valuable input, and we look forward to seeing more of your contributions in the future!

We would like to invite you to join the community chats and forums to meet other Jenkins contributors 😊
Don't forget to check out the participation page to learn more about how to contribute to Jenkins.