Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

[FIX]: Team Repo Removed from list on backspace #512

Merged
merged 3 commits into from
Aug 13, 2024

Conversation

VipinDevelops
Copy link
Contributor

@VipinDevelops VipinDevelops commented Aug 8, 2024

Linked Issue(s)

#510

Acceptance Criteria fulfillment

  • Does Not Impact Existing behavior of the Input
  • Existing Repos don't get deleted from the team repos list.

Proposed changes (including videos or screenshots)

Screencast.from.2024-08-08.22-41-30.webm

Screenshot from 2024-08-08 23-09-40

@VipinDevelops
Copy link
Contributor Author

VipinDevelops commented Aug 8, 2024

Hey @samad-yar-khan , @jayantbh

I fixed the issue. I started by looking into the onChange handler and the search query. I checked useTeamConfig thoroughly, and everything was perfectly fine there. I tried running multiple debugging sessions but found nothing.

It hit me that maybe there was something wrong with the Autocomplete component internally—perhaps a default behavior or something—because it was just deleting the last value from the repos and that to when Back Space is pressed.

I ran a few experiments in my personal project to confirm this theory, and indeed I was correct. I found that the behavior of the backspace key in the Autocomplete component (when using multiple values) is such that pressing backspace will delete the rightmost selected item.

The solution was to override or compose key-down events in Autocomplete. I just prevented it from propagating, and it worked.

@VipinDevelops VipinDevelops changed the title Fix/backspace [FIX]: Team Repo Removed from list on backspace Aug 8, 2024
@jayantbh
Copy link
Contributor

jayantbh commented Aug 8, 2024

@VipinDevelops this fix feels hacky.
I expect the issue lies in some other event handler logic going wrong.

Could you check if there's a better way to fix this?

@VipinDevelops
Copy link
Contributor Author

VipinDevelops commented Aug 8, 2024

Hey @jayantbh I agree this looks a bit hacky but I have Tried and Tested everything in the logic and there is nothing wrong, I was also thinking the same that there might be some state change issue with a key press or something but no logic is correct it just a default behavior (It took me some time to figure it out).

Here is a Issue Link that talks about the same issue in MUI itself and although it's a bit hacky I think the only why because one of the maintainers suggested it.

so this fix is like what we do for forms e.preventDefault to avoid the default behavior of forms, allowing us to handle the form submission with JavaScript and update the UI or perform other actions without refreshing the page.

@jayantbh
Copy link
Contributor

jayantbh commented Aug 8, 2024

Well... I see. In that case, the comments later in the thread suggest handling for other keys as well. Maybe you could test and handle for those as well?

@VipinDevelops
Copy link
Contributor Author

VipinDevelops commented Aug 9, 2024

Sure @jayantbh I will test out and update the Delete and ArrowLeft Keys as well.
Sounds good?

@jayantbh
Copy link
Contributor

jayantbh commented Aug 9, 2024

@VipinDevelops sounds good

@VipinDevelops
Copy link
Contributor Author

Screencast.from.2024-08-10.03-49-32.webm

Hey @jayantbh I tested it out we have no problem with Delete or Left Arrow in our case

@jayantbh jayantbh merged commit 00765dd into middlewarehq:main Aug 13, 2024
1 check passed
@jayantbh
Copy link
Contributor

Hey @VipinDevelops, I've merged it. Congrats on closing this one!

@VipinDevelops
Copy link
Contributor Author

Thanks Jayant ☺️

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

Successfully merging this pull request may close these issues.

2 participants