fix(uiSelectCtrl): scroll to dropdown position on dropdown open #2034

kidroca · 2017-07-21T11:37:47Z

Related to issue #976 and pull request #1742

As suggested in the above references - a couple of helper function were extracted
Updated the animation handling to use $q promise and resolve when the animation is unsubscribed
Then the caller triggers the actual focusing/scrolling logic so it stays out of the animation handler context
If it's not desired to use promise there the last commit can just be reverted/removed

Added tests about the scrolling functionality, the one that handles the animation callback is a bit tricky and can be updated, I haven't used $animate and written tests for it before

Had to include the select.css to karma.config in order to have proper sizes for the
elements that are tested otherwise container.scrollTop is always 0
This did not alter execution time or results for other tests

aaronroberson · 2017-12-05T19:22:13Z

Can you update your branch to resolve conflicts?

kidroca · 2017-12-06T23:47:22Z

I'll give it a go tomorrow

aaronroberson · 2017-12-07T00:05:24Z

Right on, thanks!

…ogic to private function Extracted: - method: _displayDropdown(initSearchValue, avoidReset) -> handle opening - method: _canAnimate(element) - method: _animateDropdown(searchInput, initSearchValue, container)

When ui-select is configured with `resetSearchInput: false` `ctrl.activeIndex` is not calculated when `ctrl.activate()` is invoked Opening an ui-select control with preselected model doesn't trigger active index update/reset and so the container is scroll the the element with 0 `ctrl.activeIndex` initial value (check `_ensureHighlightVisible()`)

Extracted repeating logic that focuses the search input after the dropdown is clicked Performed with $timeout so any digest or animation logic is waited for

…unction with $animate The function `_ensureHighlightVisible()` that scrolls to the active (highlighted) element is never invoked at the end of the animation, when animation is used The logic that handles non animation opening is a bit different in that it scrolls to the highlighted element Updated the animation handler to perform similarly

This sets proper element size during testing And doesn't break the rest of the test Tests for scroll position depend on these styles

…urn promise The promise is resolved when the animation is switched off (unsubscribed) This extracts the additional responsibility to call focus/scroll the active element at the end of the animation

kidroca · 2017-12-07T15:52:19Z

Ready

This PR changes uiSelectCtrl's ctrl.activate method (mostly), by extracting logic to subroutines, which could result in conflicts with some of the other pull requests that change the same method. Conflicts should be easy to fix though, because I only extracted methods and didn't change logic, besides the one needed for the fix

aaronroberson · 2017-12-07T16:16:04Z

Whoa! I no longer have permissions to merge. @Jefiozie can you review/merge this PR?

aaronroberson · 2017-12-07T17:05:20Z

To release a new version, please follow these steps:

Bump the version
Run gulp recommendedBump to increment the version in package.json and elsewhere.
Update the changelog
Run gulp changelog to update the changelog based on the conventional commit messages. You may want to visually inspect the changelog after to ensure that it was generated as expected.
Add and commit your changes
The gulp commands are not working as-is for these operations. Manually run git add . && git commit -am "chore(release): bump package version and update changelog".
Tag the commit with the package version
Either run gulp tag or manually view the new version in package.json and run git tag vX.X.X where X.X.X is the version number.
Push the commits and tag to remote
Either run gulp push or manually run git push && git push origin master --tags if the gulp command doesn't work.
Publish to npm
I don't recall if Travis CI is working in regard to auto-publishing new versions. If upon merge and push, Travis doesn't publish a new version to npm, you can publish it manually. To do so, run npm publish after logging into npm via npm login and entering your npm credentials.

All of this is supposed to be encapsulated in a single gulp command gulp bump. With a couple minor updates, this can be accomplished.

kickergold · 2018-11-06T13:06:05Z

Is this still merged? Currently this doesn't work anymore, and it doesn't scroll to the selected option.

kidroca mentioned this pull request Oct 25, 2017

Opening the dropdown would not scroll to the selected item #2070

Open

kidroca added 8 commits December 7, 2017 16:26

refactor(uiSelectCtrl): extract ctrl.activeIndex reset logic

161ecb0

refactor(uiSelectCtrl): extract ctrl.activate extract dropdown open l…

534085d

…ogic to private function Extracted: - method: _displayDropdown(initSearchValue, avoidReset) -> handle opening - method: _canAnimate(element) - method: _animateDropdown(searchInput, initSearchValue, container)

refactor(uiSelectCtrl): extract focus handling on dropdown open

3c9a3c6

Extracted repeating logic that focuses the search input after the dropdown is clicked Performed with $timeout so any digest or animation logic is waited for

test(uiSelectCtrl): test active element is scrolled into view on open

e5e93f6

chore(karma.conf): include dist/select.css

a5eb57c

This sets proper element size during testing And doesn't break the rest of the test Tests for scroll position depend on these styles

refactor(uiSelectCtrl): update _animateDropdown() to be async and ret…

e6ba734

…urn promise The promise is resolved when the animation is switched off (unsubscribed) This extracts the additional responsibility to call focus/scroll the active element at the end of the animation

kidroca force-pushed the fix-scroll-to-selected-position branch from 4191236 to e6ba734 Compare December 7, 2017 15:30

kidroca mentioned this pull request Dec 12, 2017

Opening the dropdown should scroll to the selected item. #976

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(uiSelectCtrl): scroll to dropdown position on dropdown open #2034

fix(uiSelectCtrl): scroll to dropdown position on dropdown open #2034

Uh oh!

kidroca commented Jul 21, 2017 •

edited

Loading

Uh oh!

aaronroberson commented Dec 5, 2017

Uh oh!

kidroca commented Dec 6, 2017

Uh oh!

aaronroberson commented Dec 7, 2017

Uh oh!

kidroca commented Dec 7, 2017 •

edited

Loading

Uh oh!

aaronroberson commented Dec 7, 2017

Uh oh!

aaronroberson commented Dec 7, 2017 •

edited

Loading

Uh oh!

kickergold commented Nov 6, 2018

Uh oh!

Uh oh!

fix(uiSelectCtrl): scroll to dropdown position on dropdown open #2034

Are you sure you want to change the base?

fix(uiSelectCtrl): scroll to dropdown position on dropdown open #2034

Uh oh!

Conversation

kidroca commented Jul 21, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronroberson commented Dec 5, 2017

Uh oh!

kidroca commented Dec 6, 2017

Uh oh!

aaronroberson commented Dec 7, 2017

Uh oh!

kidroca commented Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aaronroberson commented Dec 7, 2017

Uh oh!

aaronroberson commented Dec 7, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kickergold commented Nov 6, 2018

Uh oh!

Uh oh!

kidroca commented Jul 21, 2017 •

edited

Loading

kidroca commented Dec 7, 2017 •

edited

Loading

aaronroberson commented Dec 7, 2017 •

edited

Loading