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

checkNetworkQualityCatchAllConfiguration Triggers on Second Page Instead of First #56

Closed
rougegoat opened this issue May 10, 2023 · 6 comments
Assignees
Labels
enhancement New feature or request on-hold On-hold, pending external waiting feedback Waiting on feedback from requestor
Milestone

Comments

@rougegoat
Copy link
Contributor

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):

  • OS version (i.e., 13.1) 13.3.1 (a)
  • Script version (i.e., 1.6.0) - 1.10.0

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.

@rougegoat rougegoat added the bug Something isn't working label May 10, 2023
@dan-snelson
Copy link
Collaborator

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 dan-snelson added enhancement New feature or request and removed bug Something isn't working labels May 10, 2023
@dan-snelson
Copy link
Collaborator

@rougegoat:

Here's what I'm seeing:

SYM.Issue.56.mp4

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

@dan-snelson dan-snelson added this to the 1.10.1 milestone May 11, 2023
@dan-snelson
Copy link
Collaborator

@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.)

@dan-snelson dan-snelson added invalid This doesn't seem right waiting feedback Waiting on feedback from requestor and removed invalid This doesn't seem right labels May 11, 2023
@rougegoat
Copy link
Contributor Author

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
Copy link
Collaborator

Greetings, @rougegoat!

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

Thanks.

@dan-snelson dan-snelson added the on-hold On-hold, pending external label May 12, 2023
@dan-snelson
Copy link
Collaborator

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

@dan-snelson dan-snelson closed this as not planned Won't fix, can't repro, duplicate, stale May 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request on-hold On-hold, pending external waiting feedback Waiting on feedback from requestor
Projects
None yet
Development

No branches or pull requests

2 participants