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

"Close" button on export-in-progress dialog should be disabled until export is complete #291

Closed
jhgarrison opened this issue Dec 15, 2021 · 7 comments · Fixed by #293
Closed

Comments

@jhgarrison
Copy link

I accidentally clicked the "Close" button during an export and it caused a VB error.

Subsequent export attempts failed with a "write error" to Export.log. To recover I had to delete Export.log.

@hecon5
Copy link
Contributor

hecon5 commented Dec 16, 2021

Related: when done exporting/building, maybe consider an option to show the import/export buttons instead of closing. There are a lot of times when I export, I check some things on git, commit them, make a tweak, then immediately want to export again. Having to close/reopen the tool is sometimes tedious.

I do agree; I've been caught by that several times.

The trick is that we need to have a way to ensure the close is re-enabled; if there's a bug and the export / import stops, there's a chance the button will be disabled.

I think a better way to handle the close action is to add a verification and flag while an operation is in progress, and prompt the user. This way, even if there was a build/export bug and the operation stopped, and for some reason the flag isn't reset, a user can still close the form and start over.

@hecon5
Copy link
Contributor

hecon5 commented Dec 16, 2021

Actually, I think the issue is that Form_Unload isn't checking the same way that cmdClose_Click does. They should act the same, and I'm pretty sure that's what the hangup is.

@hecon5
Copy link
Contributor

hecon5 commented Dec 17, 2021

I just made a PR to fix this; @jhgarrison, and @joyfullservice can you test this and see if it fixes the issue?

All I did was force the form to use the unload (which is cancellable), I think what was happening is that the log was being set inactive, but didn't actually clean out, and the export/build operation was not killed; this obviously caused issues and isn't desirable.

Thanks!

@hecon5
Copy link
Contributor

hecon5 commented Dec 25, 2021

Found it! This is related to #247, it looks like the close action didn't get handled the same way; the issues are related; but since Access doesn't handle the Close window X the same as DoCmd.Close acForm for whatever reason they get handled differently.

@joyfullservice
Copy link
Owner

I am having trouble reproducing this issue on my end... If I am in the process of an export and click the cancel button, I get a message asking me if I really want to cancel...

image

This is the expected behavior. Some users might intuitively click the close button in their attempt to stop the process. The additional coding around the close button is what enables us to show the confirmation message before the main form disappears.

If I understand this PR correctly, it would change this behavior so that the main form disappears and would not give the user the opportunity to resume the process. Am I missing something here? What is the desired effect of the proposed PR?

@hecon5
Copy link
Contributor

hecon5 commented Feb 16, 2022

So, there are two close buttons, one is the literal "Close" button. The other is the form "X" button.

The Close button first prompts if you want to close, then closes the form. This will initiate the Form_Unload event.
That's the part that was removed. Now it just closes the form. Closing the form will trigger the Form_Unload event.

The "X" button simply closes the form. This fires the Form_Unload event (as expected).

Form_Unload already checks to confirm and cancels the close if it wasn't intended, so there's no reason to handle this twice with the same operation. There is also a generated race condition when you do it this way, because the log will bail out on unload, and then won't be active, so only handling the close cancel once (in the same place) will avoid this. The race condition is causing (I suspect) the issue because the log is "closed", but it also didn't release the file lock.

@joyfullservice
Copy link
Owner

@hecon5 - I finally had a chance to test this out this afternoon, and you are right. The form stays visible and allows you to resume the operation when prompted. I will move forward with the PR you prepared. Thanks!!

joyfullservice pushed a commit that referenced this issue Feb 23, 2022
(cherry picked from commit 6a3826bc3a48cd7749473b74c056adb31faf172e)
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 a pull request may close this issue.

3 participants