Skip to content

Fix iOS Checkboxes#438

Merged
Martinski4GitHub merged 2 commits intodevfrom
ExtremeFiretop-Test
Apr 3, 2025
Merged

Fix iOS Checkboxes#438
Martinski4GitHub merged 2 commits intodevfrom
ExtremeFiretop-Test

Conversation

@ExtremeFiretop
Copy link
Owner

@ExtremeFiretop ExtremeFiretop commented Apr 2, 2025

@Martinski4GitHub

As you know; I have no devices to test this change, but I think the issue is the fact that for the "Enable Automatic Backups" checkbox we manually set an inline style that sets its opacity when disabled. For example, in the InitializeFields() function we have:

autobackupEnabled.style.opacity = '1'; // Fully opaque
autobackupEnabled.style.opacity = '0.5'; // Grayed out appearance

But for every other checkbox, we don't set it that way and instead the other checkboxs like the days-of-the-week checkboxes or the changelog approval, the code only changes their disabled state via:

jQuery’s .prop('disabled', true).

So to try and resolve this I attempted to standardize the way we disable them all using both jQuery the DOM property.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Apr 2, 2025

@Martinski4GitHub

This is a first, I know you have access to an iPad, and I don't so I'm fully relying on you to test this one; I'm blind on my side and have only a hunch that this may work since I can't test it. Sorry buddy! ;)

Edit: BTW if it doesn't solve it, just feel free to close the PR. Was a shot in the dark kinda thing.

@Martinski4GitHub
Copy link
Collaborator

@Martinski4GitHub

As you know; I have no devices to test this change, but I think the issue is the fact that for the "Enable Automatic Backups" checkbox we manually set an inline style that sets its opacity when disabled. For example, in the InitializeFields() function we have:

autobackupEnabled.style.opacity = '1'; // Fully opaque
autobackupEnabled.style.opacity = '0.5'; // Grayed out appearance

But for every other checkbox, we don't set it that way and instead the other checkboxs like the days-of-the-week checkboxes or the changelog approval, the code only changes their disabled state via:

jQuery’s .prop('disabled', true).

So to try and resolve this I attempted to standardize the way we disable them all using both jQuery the DOM property.

My apologies, bud. I just read the messages about your latest PR for the iPad checkboxes' visual behavior. I'm on the bed with my 13" laptop, looking at my secondary email inbox (where I get the emails from SNB Forums, GitHub, and other websites that are not in my primary emails).

I briefly looked at the code changes, and I think you have a very good point about using the "style.opacity" property to gray out the checkboxes. That may actually fix the cosmetic problem on the iPadOS with Safari.

My wife is currently using the iPad, so tomorrow evening, I'll take a closer look, test & validate. I have an engineering meeting tomorrow morning at 7:00, so I'm going to bed early tonight.

I'll let you know the results. Have a good night, bud.

@Martinski4GitHub
Copy link
Collaborator

@Martinski4GitHub

As you know; I have no devices to test this change, but I think the issue is the fact that for the "Enable Automatic Backups" checkbox we manually set an inline style that sets its opacity when disabled. For example, in the InitializeFields() function we have:

autobackupEnabled.style.opacity = '1'; // Fully opaque
autobackupEnabled.style.opacity = '0.5'; // Grayed out appearance

But for every other checkbox, we don't set it that way and instead the other checkboxs like the days-of-the-week checkboxes or the changelog approval, the code only changes their disabled state via:

jQuery’s .prop('disabled', true).

So to try and resolve this I attempted to standardize the way we disable them all using both jQuery the DOM property.

You hit the nail right on the head, bud!! 👍
Setting the "style.opacity" property was required to make the iPadOS/Safari checkboxes behave the same way as on Windows WRT their visual presentation when set to the disabled state. Great catch!!

Here's a screenshot from my iPad Pro 11 now:

MerlinAU_v1 4 0 _WebGUI_iPadP11_OK

Copy link
Collaborator

@Martinski4GitHub Martinski4GitHub left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great catch!! Approved!!

@Martinski4GitHub Martinski4GitHub merged commit b795cc7 into dev Apr 3, 2025
2 checks passed
@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Apr 3, 2025

Great catch!! Approved!!

Whooooohooooooooooo!!!!!
Small victories but I'll take em! Took some poking to as to why it worked for the auto backup option, but once I found that was the only setting using those values I figured it was worth a shot in the dark lol.

Happy it works! We should increase the build timestamp I just wasn't sure if it was going to be merged

@Martinski4GitHub
Copy link
Collaborator

Great catch!! Approved!!

Whooooohooooooooooo!!!!! Small victories but I'll take em! Took some poking to as to why it worked for the auto backup option, but once I found that was the only setting using those values I figured it was worth a shot in the dark lol.

And a great shot it was!! Certainly worth it!!

Happy it works! We should increase the build timestamp I just wasn't sure if it was going to be merged

Yep, I'll do that shortly with the upcoming PR that does some cleanup.

@ExtremeFiretop
Copy link
Owner Author

ExtremeFiretop commented Apr 3, 2025

Great catch!! Approved!!

Whooooohooooooooooo!!!!! Small victories but I'll take em! Took some poking to as to why it worked for the auto backup option, but once I found that was the only setting using those values I figured it was worth a shot in the dark lol.

And a great shot it was!! Certainly worth it!!

Happy it works! We should increase the build timestamp I just wasn't sure if it was going to be merged

Yep, I'll do that shortly with the upcoming PR that does some cleanup.

Awesome! It's already pretty late for me, so I'm heading to bed now. I'll probably be up for another little bit by just feel free to merge in the cleanup at will to dev. Otherwise if something actually changed I'll review in the morning!

Goodnight buddy thanks for testing I appreciate you!

@Martinski4GitHub
Copy link
Collaborator

Great catch!! Approved!!

Whooooohooooooooooo!!!!! Small victories but I'll take em! Took some poking to as to why it worked for the auto backup option, but once I found that was the only setting using those values I figured it was worth a shot in the dark lol.

And a great shot it was!! Certainly worth it!!

Happy it works! We should increase the build timestamp I just wasn't sure if it was going to be merged

Yep, I'll do that shortly with the upcoming PR that does some cleanup.

Awesome! It's already pretty late for me, so I'm heading to bed now. I'll probably be up for another little bit by just feel free to merge in the cleanup at will to dev. Otherwise if something actually changed I'll review in the morning!

There are some changes to remove extra code that was disabling the checkboxes two or three times in a row. Now it's done only once as needed. There's no rush, so you can review it tomorrow if you want. I have tested and validated this PR on my Windows and iPadOS devices.

Goodnight buddy thanks for testing I appreciate you!

Of course, bud. That's what team effort is all about!!
Have a good night & sleep tight, bud!!

@ExtremeFiretop ExtremeFiretop deleted the ExtremeFiretop-Test branch April 5, 2025 02:14
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.

2 participants