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

Add custom install option #401

Merged
merged 36 commits into from May 23, 2023

Conversation

hecon5
Copy link
Contributor

@hecon5 hecon5 commented May 18, 2023

This is a rebuild of #361 now that the fork was split.

…-archive for more information on how to make archives easily.
… location is crucial to our being able to use it.
…ing this built in several places, get install location from one common Function.
…der (for hotfixing and also troubleshooting).
…lOption

# Conflicts:
#	Version Control.accda.src/forms/frmVCSOptions.bas
#	Version Control.accda.src/modules/modInstall.bas
…lOption

# Conflicts:
#	Version Control.accda.src/forms/frmVCSOptions.bas
#	Version Control.accda.src/modules/modComAddIn.bas
#	Version Control.accda.src/vcs-options.json
@hecon5
Copy link
Contributor Author

hecon5 commented May 18, 2023

@joyfullservice This is ready for checking and inclusion; If you would be so kind as to take a peek and merge or let me know if there are other fixes, that's be great, I'd like to have my team start using 4.x, and this will let them do it.

@joyfullservice
Copy link
Owner

Thanks for your work on this! There are a few tweaks I will probably make, (which has been the main holdup for me) but I will see if I can get this worked through in the next several days. I am excited about your team getting to move to version 4. 😄

@hecon5
Copy link
Contributor Author

hecon5 commented May 18, 2023

Super, looking forward to it!

@joyfullservice
Copy link
Owner

Do you mind if I commit directly to this PR? I just want to make sure that when I merge it into dev, it is ready to go and doesn't leave anything hanging out there that might trip up other users that are regularly building the dev branch.

@hecon5
Copy link
Contributor Author

hecon5 commented May 18, 2023

No; I think that'll be fine; will also let me see what you change (curiosity's sake), and I'm personally a fan of avoiding commits directly to dev anyway.

Unless there is a known conflict or other reason to use prefixes on function names, I prefer to use the simpler declarative names to make the source code more readable.
(Testing to ensure that I can successfully write to this PR branch.)
Using a tab control allows us the simplicity of a one-click install with no extra potentially confusing options displayed, but gives a one-click navigation path to see the additional options.
@joyfullservice
Copy link
Owner

I made some progress on this today, and will plan to pick it up again as I have opportunity next week. As you will see in the following screenshots, I spent most of my time on the user interface, changing the install form to use a tab control so we can optionally display the more advanced installation settings. This helps keep things very simple for most users, while still providing those more advanced options for those that need/require those.

image

From the basic install page, you simply click the navigation link to switch to the advanced install screen.

image

Again, I haven't wired these up to the rest of the code, but I like the cleaner user experience with this approach.

@joyfullservice
Copy link
Owner

I reworked the UI a little further to use a checkbox instead of the navigation link. This seems to look nicer from a design standpoint and is possibly slightly more intuitive for the user.

image

image

@hecon5
Copy link
Contributor Author

hecon5 commented May 22, 2023

I like that better, too. Cleaner.

I was thinking about that "trust Addin folder" option; should we move that to the install options, and just call it "Install Options" instead of "advanced"? That way, the default options are selected normally, but the user can alter them, just like an actual installer?

@joyfullservice
Copy link
Owner

I was thinking about that "trust Addin folder" option; should we move that to the install options, and just call it "Install Options" instead of "advanced"? That way, the default options are selected normally, but the user can alter them, just like an actual installer?

That's a good consideration. Here is my thought process on the existing setup...

For a new user that has never used the add-in before, this is their first introduction to the system. They probably don't have the level of trust that a long-time user would have. As we work to earn their trust, one part of that is being transparent about any security related settings. On first install, they are going to see a confirmation box requiring their consent to trust the add-in folder. The fact that they have already seen a checkbox with that option lends some credibility to the confirmation. This is something they were already aware of, and just confirming it. In my thinking, this builds trust.

image

On the other hand, if the user doesn't see that option and clicks to install, they will not have the additional subtle context for the confirmation dialog, and it will be slightly more likely to surprise the user and potentially raise concerns about what other security settings might be affected. This is one setting where I feel it is helpful for the user to be aware of, even on a basic default install.

Using a checkbox to toggle the advanced options instead of a hyperlink navigation aid. joyfullservice#401
@hecon5
Copy link
Contributor Author

hecon5 commented May 22, 2023

Fair point; that was actually the consideration I had at first, but wasn't sure if "very clean" or "clean" was the way to go.

I'm convinced it's good as is.

@joyfullservice
Copy link
Owner

Made a bunch of progress on this today. Hoping to finish it up tomorrow...

@joyfullservice
Copy link
Owner

Okay, I have finished a pretty significant rework of the installer system. You will see a bunch of commits coming through as I document various changes...

As we add more settings and support the ability to install in alternate locations, using a private type becomes a more manageable and scalable solution.
Removing some constants that we no longer need after reworking the installer.
Cleaning up unused functions and legacy RC4 checks.
This keeps the code more organized by keeping the install/uninstall logic within the modInstall module. This way we can keep most of the functions private where they won't conflict with other parts of the application.
This is more related to the UI side, and not reused anywhere else.
Split out some of the logic into functions, worked through some bugs.
When you change to a different install folder, you now have the option of keeping your current settings even through the uninstall/reinstall process.
Affected by some refactoring done with the installer to allow a custom installation path. joyfullservice#397
This function had shared the same name as the public function in the installer. While this worked because of the overloading functionality where the private function defined in this module took precedence over the global function, it could really cause confusion down the road! I renamed the function to explicitly clarify which add-in we are talking about.  😄
We have refactored the installer so that the ribbon should not be installed or launched if that option was not selected in the installer.
Pulling from the refactored install settings.
The worker class gives us the ability to remove the actual database add-in file and related lock file during the uninstall process. The worker script is then able to delete itself, allowing us a clean uninstall of the add-in files. (The worker creates a vbscript file that runs independently of the Access VBA code, but can interact with the add-in for some operations. We don't use it a lot at this point, but it may be used more in the future.)
@joyfullservice joyfullservice merged commit 5076bb4 into joyfullservice:dev May 23, 2023
@hecon5 hecon5 deleted the AddCustomInstallOption branch May 23, 2023 14:42
@hecon5
Copy link
Contributor Author

hecon5 commented May 23, 2023

Cool, I'm going to build this and let you know, thanks!

@joyfullservice
Copy link
Owner

Sounds good! Make sure you catch that additional commit in dev to fix the options json file.

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.

None yet

2 participants