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

Bug Fix: Delete feature (Fixes three issues) #3383

Merged
merged 8 commits into from
Jun 13, 2023

Conversation

CertifiedJoon
Copy link
Contributor

@CertifiedJoon CertifiedJoon commented Jun 11, 2023

Fixes: #3267
Fixes: #3306
Fixes: #3378

Regarding: #3267
Some of the z-indices were manged by zIndex parameter, while other components where managed by Portals. Somehow the two conflicted and created a weird mix of zIndices. I have changed popover's portal into simple zindex parameter and parents, sibligns, and cousins accordingly.

Regarding: #3306
The redirection to deleted chat happened because one of its parent elements were a Link component. Previously, upon pressing delete, stopEvent() was called to temporarily pause redirection, and redirection would resume after the delete process. I have changed the Link component into normal button with onClick redirection, maintaining the stopPropogation behavior.

Regarding: #3378
The default behavior of AlertDialog would wrap the text. But somehow using portal on it would restrict text-wrap behavior. The text-wrap behavior came back after removal of portal while working on #3267 .

deleting from /chat/{id}
image

deleting from /chat
image

toast on delete
image

@CertifiedJoon CertifiedJoon marked this pull request as ready for review June 11, 2023 13:24
@notmd
Copy link
Collaborator

notmd commented Jun 11, 2023

Thanks, but I believe you are relying on zIndex too much which will be hard to debug in future. I will take a look and get rid of it.

@CertifiedJoon
Copy link
Contributor Author

CertifiedJoon commented Jun 11, 2023

Thanks, but I believe you are relying on zIndex too much which will be hard to debug in future. I will take a look and get rid of it.

Thank you :) I've made the two changes you have mentioned.

@notmd notmd self-assigned this Jun 11, 2023
@github-actions
Copy link

pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

@notmd
Copy link
Collaborator

notmd commented Jun 12, 2023

I just push all the fix, please check @CertifiedJoon

@github-actions
Copy link

pre-commit failed.
Please run pre-commit run --all-files locally and commit the changes.
Find more information in the repository's CONTRIBUTING.md

@CertifiedJoon
Copy link
Contributor Author

@notmd Thank you for your help! Looks great. I will make sure to use portals more often from now on.

@notmd notmd merged commit 1f0113f into LAION-AI:main Jun 13, 2023
@CertifiedJoon CertifiedJoon deleted the issue/3378 branch June 13, 2023 07:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants