Avalonia icon indicating copy to clipboard operation
Avalonia copied to clipboard

Can't properly disable focused controls

Open luthfiampas opened this issue 2 years ago • 11 comments

Describe the bug Some key events are still fully functional in disabled+focused controls. Probably related to https://github.com/AvaloniaUI/Avalonia/issues/6957, but the PR does not solves other controls. With Fluent theme, control style don't get updated. But I can see in the debugger that IsEnabled is already set to false.

To Reproduce

  1. Make TextBox focus
  2. Disable the TextBox
  3. Start typing..

FocusedControlsIssue.zip

Expected behavior Disabled controls should not accept key events.

Screenshots

https://user-images.githubusercontent.com/57059775/147838556-cb1a0b3b-227a-4784-ae87-041aa0ad19d0.mp4

Desktop (please complete the following information):

  • OS: Debian (dwm)
  • Version: 0.10.10

luthfiampas avatar Dec 31 '21 20:12 luthfiampas

I wonder what WPF does in this situation. Expected would be to unfocus element, but it's not clear what should be focused instead.

maxkatz6 avatar Dec 31 '21 21:12 maxkatz6

@maxkatz6 In WPF, neither IsFocused property or FocusManager.GetFocusedElement was changed. I will try to take a deeper look.

luthfiampas avatar Jan 01 '22 08:01 luthfiampas

My understanding is at a framework level keyboard events should not be passed to disabled controls. That would solve the issues I see in the screen recording. It also shouldn't be too difficult to implement. That said, I seem to recall someone in the past intentionally enabled this (probably incorrectly).

robloo avatar Jan 01 '22 18:01 robloo

Seems to be a fairly fundamental issue. I'm surprised it has so far gone unnoticed except for the special cases. I see lots of examples:

https://github.com/AvaloniaUI/Avalonia/pull/6632 https://github.com/AvaloniaUI/Avalonia/pull/7206

robloo avatar Jan 02 '22 02:01 robloo

I wonder what WPF does in this situation. Expected would be to unfocus element, but it's not clear what should be focused instead

Here is a really nice write-up: https://wpf.2000things.com/tag/isenabled/

"A user interface element must be both visible (Visibility = Visible) and enabled (IsEnabled = true) in order to fire an event. If an element is not visible or is disabled, the topmost element beneath the element may fire the event instead."

robloo avatar Jan 02 '22 02:01 robloo

My understanding is at a framework level keyboard events should not be passed to disabled controls. That would solve the issues I see in the screen recording. It also shouldn't be too difficult to implement. That said, I seem to recall someone in the past intentionally enabled this (probably incorrectly).

Ignoring keyboard events in base element will definitely solves the issue. However, as we can see in the screen recording, TextBox's caret and focus adorner still rendered. Also, some of disabled controls in Fluent theme doesn't get proper style if focused. So I don't think this will be enough, not sure if this is also related to style specification issue.

The quick fix is probably as @maxkatz6 said, we can unfocus element if it gets disabled, but I'm not sure if this will be the correct approach. As far as I know, WPF don't do this.

I'm thinking to change this.. https://github.com/AvaloniaUI/Avalonia/blob/e27bc249d30c1df150dfb3fa58e91ef888b58a93/src/Avalonia.Input/InputElement.cs#L451-L459

..to something like this

public bool IsEffectivelyEnabled
{
    get => _isEffectivelyEnabled;
    private set
    {
        SetAndRaise(IsEffectivelyEnabledProperty, ref _isEffectivelyEnabled, value);
        PseudoClasses.Set(":disabled", !value);

        if (!IsEffectivelyEnabled)
        {
            if (FocusManager.Instance?.Current == this)
            {
                _restoreFocus = true;
                FocusManager.Instance?.Focus(null);
            }
            else
            {
                _restoreFocus = false;
            }
        }
        else if (IsEffectivelyEnabled && _restoreFocus)
        {
            FocusManager.Instance?.Focus(this);
        }
    }
}

luthfiampas avatar Jan 02 '22 08:01 luthfiampas

we can unfocus element if it gets disabled, but I'm not sure if this will be the correct approach. As far as I know, WPF don't do this.

This should be checked. I have the opposite impression. I know once a control is disabled it cannot be focused. However, if it was currently focused and then disabled I suppose that could be a special case. I would guess WPF removes focus and returns to default as if window was first loaded though.

Also note:

  1. Best practice is for the app to move focus from disabled controls before disabling them
  2. Newer frameworks have AllowDisabledFocus property. This seems under debate but the reasoning is disabled controls may still need to be focusable for accessibility -- although they still receive no input.

I'm inclined to say we need to support point 2 in a modern framework.

robloo avatar Jan 02 '22 16:01 robloo

In WPF a control is no longer interactive when it is disabled (IsEnabled=false). The focus will be lost. This can also lead to a null value for the current focused element when the current keyboard focus "holder" gets disabled by a binding etc.

I am dealing with a similar issue in Avalonia. I have disabled controls but unfortunately I can still access them via Ctrl+Tab including changing is values via keyboard. In my opinion this is a bug. Why should I disable the control in the first place when interacting with them is still possible?

Avalonia works correct when the "IsEnabled" state is set in the AXAML file directly. Then no interaction is possible. But setting it later (or at container level) will lead to a "half disabled" control. Visually it is but from interaction perspective it is not.

chkr1011 avatar Feb 28 '22 12:02 chkr1011

Visually it is but from interaction perspective it is not

Yep, tree inheritanсe doesn't work(

kirichenec avatar Mar 21 '22 21:03 kirichenec

Yes, this is definitely a bug 😕

SuperJMN avatar May 25 '22 21:05 SuperJMN

Same issue still happens if you are typing in a TextBox with IsEnabled="{Binding !IsLoading}", and a Button has IsDefault="True" and sets IsLoading = false;. You can continue typing in the TextBox until you change focus.

Here is some test code: (click to expand)
public partial class FormTest : UserControl {
    public FormTest() {
        SubmitCommand = ReactiveCommand.Create(
            async () => {
                IsLoading = true;
                await Task.Delay(5000);
                IsLoading = false;
                return;
            },
            this.ObservableForProperty(x => x.IsLoading, skipInitial: false).Select(x => !x.Value)
        );
        this.DataContext = this;
        AvaloniaXamlLoader.Load(this);

    }

    public ReactiveCommand<Unit, Task> SubmitCommand { get; }

    public static readonly StyledProperty<bool> IsLoadingProperty =
        AvaloniaProperty.Register<FormTest, bool>(nameof(IsLoading), false);

    public bool IsLoading {
        get { return GetValue(IsLoadingProperty); }
        set { SetValue(IsLoadingProperty, value); }
    }

}
<UserControl xmlns="https://github.com/avaloniaui"
             xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
             xmlns:d="http://schemas.microsoft.com/expression/blend/2008"
             xmlns:mc="http://schemas.openxmlformats.org/markup-compatibility/2006"
             mc:Ignorable="d" d:DesignWidth="200" d:DesignHeight="200"
             x:Class="AvaloniaFindIssue.Views.FormTest">
	<Border Padding="20">
		<StackPanel>
			<TextBox IsEnabled="{Binding !IsLoading}" Watermark="Name"></TextBox>
			<TextBox IsEnabled="{Binding !IsLoading}" Watermark="Email"></TextBox>
			<Button Command="{Binding SubmitCommand}" IsDefault="True">Submit</Button>
		</StackPanel>
	</Border>
</UserControl>

Start typing in one field, click Enter and you can still type more characters.

RationalFragile avatar Jul 29 '22 17:07 RationalFragile

So far there are two special cases where disabled controls should potentially be interacted with:

  1. Some newer frameworks have AllowDisabledFocus property. This seems under debate but the reasoning is disabled controls may still need to be focusable for accessibility (think screen readers) -- although they still receive no input.
  2. Sometimes you may want to show a special cursor over disabled controls indicating they cannot be clicked or interacted with. (#9640)

robloo avatar Dec 07 '22 01:12 robloo