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

Async delete #126

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Async delete #126

wants to merge 3 commits into from

Conversation

yunenliu
Copy link

@yunenliu yunenliu commented Aug 30, 2020

Currently, async deletes don't work because the js submits them twice (once to validate the form, once to actually perform the action). If we assume deletes don't generally require form validation, then the problem is solved by adding a setting that skips the form validation step and submits it directly.

Added async delete code alongside the standard delete code to show how to use the new setting.

Added a functional test of async delete (note that since the page never reloads, now we get an empty table instead of a message that no books are in the database)

Yun-En Liu added 3 commits August 30, 2020 09:43
…mpty table behind). Also added gecko autodriver as a requirement and imported into tests since I got an error about not having geckodriver on path when I followed the instructions
@trco
Copy link
Owner

trco commented Aug 31, 2020

@yunenliu Nice work. I'll check it out and accept it as soon as possible. Most probably in coming weekend.

@trco trco mentioned this pull request Aug 31, 2020
@trco trco self-requested a review May 2, 2023 13:18
@trco
Copy link
Owner

trco commented Jul 1, 2023

@yunenliu Sorry for not including your PR back in 2020. I'm again working on this package from time to time thus I would like to ask you if you're interested in adjusting this outdated PR to the current code. It would be great to have you as collaborator. Otherwise I would like to thank you for your contribution and include your idea in one of the next releases.

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.

2 participants