SettingsProtectionHelper: support always-hidden controls - #1522
Conversation
…ontrols (#1134) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Palaso Tests 4 files ± 0 4 suites ±0 10m 27s ⏱️ ±0s Results for commit 4fdbee4. ± Comparison against base commit e65a140. This pull request skips 1 test.♻️ This comment has been updated with latest results. |
- Restore CHANGELOG from base commit to fix double-encoded UTF-8 (Chinese characters and curly apostrophe were corrupted by PowerShell Get-Content/Set-Content mishandling UTF-8-without-BOM) - Re-apply keepHidden entry in the correct ### Added section - Add XML doc to 3-param SetSettingsProtection overloads clarifying that keepHidden has no effect when isProtected is false Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Timer is owned by the components container, so components.Dispose() already disposes it; the explicit _checkForCtrlKeyTimer.Dispose() call was a no-op double-dispose. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The keepHidden feature entry didn't cover the separate Dispose(bool) fix that moves managed-resource cleanup inside the disposing guard and removes the redundant explicit timer disposal.
- Follow the standard Dispose(bool) form used elsewhere in the repo; set _isDisposed only after base.Dispose(disposing), since Component's base implementation removes itself from its container, which reads the Site property that this class overrides to throw once disposed - Dispose test controls with using - Initialize the tracking sets in field initializers so they are non-null before anything can observe them Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
- Hide an always-hidden component as soon as it is registered instead of leaving it visible until the next timer tick, but only at run time: the extender property setter also runs at design time, where the timer is off and the component must stay visible on the design surface - Give the new SetSettingsProtection overloads a summary and tags for every parameter, and correct the reveal mechanism in the text (holding Ctrl+Shift, not the settings password) Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Registering a component as always-hidden hides it at once, so the reverse transition now applies the normal visibility rule at once too, rather than leaving the component hidden until the next timer tick. Extracted the visibility rule into a property shared with UpdateDisplay, which was recomputing it once per component. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This comment was marked as resolved.
This comment was marked as resolved.
papeh
left a comment
There was a problem hiding this comment.
@papeh reviewed 1 file and made 1 comment.
Reviewable status: 1 of 4 files reviewed, 2 unresolved discussions (waiting on imnasnainaec).
SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs line 77 at r1 (raw file):
foreach (var component in _componentsUnderSettingsProtection) { bool visible = !SettingsProtectionSingleton.Settings.NormallyHidden || ((Control.ModifierKeys & keys) == keys);
nice improvement!
imnasnainaec
left a comment
There was a problem hiding this comment.
@imnasnainaec reviewed all commit messages and made 1 comment.
Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on papeh).
SIL.Windows.Forms/SettingProtection/SettingsProtectionHelper.cs line 77 at r1 (raw file):
Previously, papeh wrote…
nice improvement!
Thanks!
Summary
SetSettingsProtection(Control, bool, bool keepHidden)andSetSettingsProtection(ToolStripItem, bool, bool keepHidden)overloads so a control or ToolStripItem can be marked always-hidden, staying hidden even while Ctrl+Shift reveals the other protected components. The original 2-parameter overloads are preserved for WinForms designer extender-property discovery and binary compatibility._alwaysHiddenComponentsset, whichUpdateDisplay()hides unconditionally after the normal pass.GetSettingsProtectionreportstruefor them too.!NormallyHidden || Ctrl+Shiftrule moved into a property shared withUpdateDisplay(), which had been recomputing it once per component.Dispose(bool)into the standard form used elsewhere in the repo — the missing-Dispose debug warning, an early return that makes it idempotent, managed cleanup only whendisposing, and_isDisposedset afterbase.Dispose(disposing). That ordering matters:Component.Disposeremoves itself from its container, which reads theSiteproperty, and this class overridesSiteto throw once disposed. The explicit_checkForCtrlKeyTimer.Dispose()is gone, since the timer is owned by thecomponentscontainer that disposes it.SettingsProtectionHelperTests— 11 tests covering normal vs. always-hidden visibility, both transition directions and their immediacy, unprotecting,GetSettingsProtection, and disposing while still sited in a container.Scope note on #1134
keepHiddenis a "never show this" primitive: an always-hidden component is not revealed by Ctrl+Shift, and it is not revealed by entering the settings password either — nothing records that the password was entered, andUpdateDisplay()re-hides these components on every tick.#1134 asks for components that remain hidden unless the password is entered. A client can build that on top of this: mark the components
keepHidden: true, then re-register them withkeepHidden: falseafter a successful challenge. This PR does not add that behavior to the library itself.Fixes #1134
Devin review: https://app.devin.ai/review/sillsdev/libpalaso/pull/1522
This change is