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: Avoid focusing on modal instead of field on specific cases #39

Closed
wants to merge 1 commit into from

Conversation

icgsbr
Copy link

@icgsbr icgsbr commented Oct 18, 2023

@antonio-ortega
Copy link
Member

antonio-ortega commented Oct 23, 2023

Hey @icgsbr ,

i've been checking this case and I don't think this is the right solution.

We are updating something really deep into our code and used massively to fix something that's only happening in Forms when using a custom DropdownListWithSearch component.

Have you tried to manage the focus in that component? I think that would be much safer.

If you have tried and see something why that could not be a solution, let me know, please.

Thanks.

Regards.

@icgsbr
Copy link
Author

icgsbr commented Oct 23, 2023

Hi @antonio-ortega !

Though this solution is not ideal, it's the one we could find that solves the reported issue.

What is going on is that some part of the code is hijacking the focus from the field and sending it to the modal; therefore, even if we try to force the focus event on DropdownListWithSearch, it will still focus on the modal since this behavior is coming from a deeper part of the code.

The solution contained in the sent commit was recommended by an engineer from frontend-infra through a PTR, which was opened after we exhausted our attempts to fix it at our side.

If you need any more information or have a better approach idea, feel free to contact me.

Best regards.

@antonio-ortega
Copy link
Member

Hi @icgsbr ,

This is a specific issue in that dropdown and it should be fixed there.

As an example on how to do it, you can see this example, just by doing that you are able to focus on search input.

It is not working on the first click, but that's because you have another issue with that dropdown because you are not focusing on it when it gets opened. But as soon as you fix that, you'll see how with that code it's enough to fix this focus issue without patching YUI.

I'm closing this pull request now, since the code I linked above proofs this can be fixed and it should be fixed within that specific component.

Thanks.

Regards.

Pd: Btw, in that commit I linked above there is a modification on MAX_ITEMS constant. Just ignore it. That's not needed at all.

@icgsbr
Copy link
Author

icgsbr commented Nov 8, 2023

Hi, Antonio Ortega!

I'm here again to ask for your help concerning this solution. Indeed, what you proposed on https://github.com/antonio-ortega/liferay-portal-ee/commit/a05c7bcfab6aee871aa9c95648dd3e79f3195384 works, yet only on the second click due to the fact the dropdown isn't being focused when opened. I tried a lot to fix the mentioned issue with help from other frontend engineers as well, but, unfortunately, I got no success in solving it. Could you please help me on this matter?

P.S: I even tried forcing events and nothing appeared to work.

@antonio-ortega
Copy link
Member

Hey @icgsbr ,

Are you sure this issue is still reproducible?

I've just compiled current master branch (commit) and follow the steps in LPP-50079 and I can no longer reproduce it, so it seems like some changes were introduced in Forms in the last two weeks fixing this.

Thanks.

Regards.

@icgsbr
Copy link
Author

icgsbr commented Aug 27, 2024

ci:close

@icgsbr icgsbr closed this Aug 27, 2024
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.

2 participants