Skip to content

Conversation

ChildishGiant
Copy link
Member

This would close #133

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • I have read the CONTRIBUTING document.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

@ChildishGiant ChildishGiant changed the title fix(dropdown): 🐛 fix a bug with dropdown actives when used in selects fix(dropdown): 🐛 fix a bug with dropdown actives when used in selects May 8, 2021
@DanielRuf DanielRuf requested a review from a team May 10, 2021 06:18
@bugy
Copy link

bugy commented May 10, 2021

I'll check the changes and test them in the evening.

@bugy
Copy link

bugy commented May 10, 2021

@ChildishGiant I tested this change and it seems, that it reverts your improvements in #98

@bugy
Copy link

bugy commented May 10, 2021

As a solution, I found this one: https://stackoverflow.com/a/48500276/2166673

It might be not supported in some browsers, but as for me, those browsers can live with a default non-smooth scroll
Or we should do scrollIntoView first and make it immediate. Otherwise focus() doesn't wait until smooth animation and scrolls immediately

@ChildishGiant
Copy link
Member Author

I remember checking that the function still worked for me so I'll have to take a look at this.

@DanielRuf
Copy link

Hey,

You might be busy but I just wanted to check if there is an update regarding the last comment.

@ChildishGiant
Copy link
Member Author

FsNV9kRi3n.mp4

I've checked again and I'm still not having issues. Could you show the issues you're having?

@bugy
Copy link

bugy commented Jun 6, 2021

Hi @ChildishGiant, sorry if I'm doing something wrong, but I still have the problem

dropdown_jump.mp4

I'm testing in Chrome, and using my own testbed + vue. I don't use compiled dist.js file, but rather dropdown.js directly. May be you could try rebuilding the dist file locally?
Otherwise, I don't know what could be the issue :(

@ChildishGiant
Copy link
Member Author

I've cloned the repo to a different folder, switched branch and built with no issues showing. Are you sure you're on the right branch and have built after changing over? If anyone else could check this that'd be great

@bugy
Copy link

bugy commented Jun 7, 2021

Alright, I found where our testing issue is: autocomplete uses custom handling for up/down keys. see autocomplete.js _handleInputKeydown
Could you try testing with a normal dropdown, please?

@ChildishGiant
Copy link
Member Author

Ah, there we go. Thanks for pointing that out. Should be all fixed on all kinds of dropdowns now.

@bugy
Copy link

bugy commented Jun 7, 2021

Thanks for the fix! But could we go with the focus() approach? It would also solve this problem (from #133):

Another problem which I see now, is that the list elements are not focused anymore. document.activeElement always returns <ul> element, instead of <li>

@ChildishGiant
Copy link
Member Author

I've updated it to the method you've linked and agree that it is better. I'm sorry for the continued back and forth about this issue. Using smooth scroll combined with the background transition does cause readability while holding to scroll to not be great, sadly. The only real option to fix this would be lower the transition time and disable smooth scroll but I think maybe it's best to keep it as is.

6piY3hHNzO.mp4

@bugy
Copy link

bugy commented Jun 7, 2021

Looks good, thanks!

@wuda-io
Copy link
Member

wuda-io commented Aug 13, 2021

I also have the issue when clicking on Dropdowns of the Select Element in the current Website. Whats the status of this fix? Looking forward for a merge. Greets

@DanielRuf DanielRuf requested a review from a team August 13, 2021 07:42
@DanielRuf DanielRuf merged commit 10c81ba into materializecss:v1-dev Aug 15, 2021
@ChildishGiant ChildishGiant deleted the select-active-fix branch September 18, 2021 23:29
@Smankusors Smankusors added the bug Something isn't working label Apr 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Options in select element still highlighted after unselecting it.
5 participants