setup-your-mac / Setup-Your-Mac

Setup Your Mac aims to simplify initial device configuration by leveraging swiftDialog and Jamf Pro Policy Custom Events to allow end-users to self-complete Mac setup post-enrollment.
https://snelson.us/sym
MIT License
249 stars 55 forks source link

checkNetworkQualityCatchAllConfiguration Triggers on Second Page Instead of First #56

Closed rougegoat closed 1 year ago

rougegoat commented 1 year ago

Before submitting a Setup Your Mac bug report, please review the open swiftDialog issues to help determine the source of the issue.

Describe the bug The download estimate for the Catch-All configuration runs on the second page, which results in it wiping out all the displayed information like Computer Name, Username, Department, etc in the infobox.

To Reproduce Set promptForConfiguration to false and configurationDownloadEstimation to true. Run as normal.

Expected behavior On the first page, you should see the download estimate show in the Infobox section for the catch-all configuration in the same way it works if promptForConfiguration is set to true.

Code/log output Please supply the full command used, and if applicable, add full output from Terminal. Either upload the log, or paste the output in a code block (triple backticks at the start and end of the code block, please!).

Screenshots These two screenshots have the same User Input entered save for the configuration, which is Required on the first and the Catch-All on the second. Screenshot of behavior if promptForConfiguration and configurationDownloadEstimation are both set to true Screenshot 2023-05-10 at 4 49 01 PM

Screenshot of behavior if promptForConfiguration is set to false and configurationDownloadEstimation is set to true Screenshot 2023-05-10 at 4 41 19 PM

Environment (please complete the following information):

Additional context I'm not sure the best route to addressing this. I think you could potentially combine both the checkNetworkQualityConfigruation and checkNetworkQualityCatchAllConfiguration, but those interact with enough things that I'm not confident in that guess.

dan-snelson commented 1 year ago

To me, @rougegoat, this is (currently) "working as designed," but let me replicate and review.

(If there's only a single Configuration, the Dynamic Download Estimates are just an FYI.)

dan-snelson commented 1 year ago

@rougegoat:

Here's what I'm seeing:

https://github.com/dan-snelson/Setup-Your-Mac/assets/24623109/ed72356c-e479-40c5-9e50-dac4d102cc13

(To me, what happens at 00:15 to 00:16 is a distraction; I'll take a look at removing that.)

dan-snelson commented 1 year ago

@rougegoat:

The visual "glitch" has been addressed in 1.10.1-b3.

(Please feel free to respond here or DM me on the MacAdmins Slack if you'd like to discuss the current approach.)

rougegoat commented 1 year ago

That half works. It addresses one issue, but then makes the infobox be stuck as Analyzing Input... if promptForConfiguration and configurationDownloadEstimation are both set to False. That could be addressed by changing line 2895 to

if [[ "${promptForConfiguration}" != "true" ]] && [[ "${configurationDownloadEstimation}" == "true" ]]; then

I think I have a slight rewrite that basically moves the if [[ "${promptForConfiguration" == "true" ]]; then bit into the checkNetworkQualityConfiguration function. That makes it so that one function can do both in the same place and removes the need for checkNetworkQualityCatchAllConfiguration and other workarounds. I'll submit a PR.

dan-snelson commented 1 year ago

Greetings, @rougegoat!

I'll mark this issue "on-hold" while we iron-out #57.

Thanks.

dan-snelson commented 1 year ago

Since #57 was just closed, closing this as well.