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

fix combo box auto scrolling on componentDidUpdate #4879

Merged
merged 4 commits into from
Jun 16, 2021

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jun 14, 2021

EuiComboBox auto scrolls to the selected item in componentDidUpdate. This makes the combo box unusable, as it auto-scrolls away from where users are looking any time there is an update. This issue is occurring in maps elastic/kibana#78420.

This PR resolves the problem by moving the call to this.listRef.scrollToItem from componentDidUpdate to within setListRef.

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4879/

1 similar comment
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4879/

Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

This breaks the ability to arrow through the options (tested in FF)

In the video first I arrow up/down the the combobox on this branch and then I switch to master and do it again. On this branch, once you arrow past the visible list, the box doesn't scroll. On master, arrowing past the visible list scrolls the box.

Kapture.2021-06-14.at.17.38.35.mp4

@chandlerprall
Copy link
Contributor

Instead of moving the logic, I think it would work as intended if we added another condition checking for props.activeOptionIndex !== prevProps.activeOptionIndex

@nreese nreese requested a review from myasonik June 14, 2021 21:55
@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4879/

Copy link
Contributor

@myasonik myasonik left a comment

Choose a reason for hiding this comment

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

Prettier error needs to be fixed but otherwise all bugs seem to be gone!

Thanks @nreese!

@kibanamachine
Copy link

Preview documentation changes for this PR: https://eui.elastic.co/pr_4879/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants