Uno.Themes icon indicating copy to clipboard operation
Uno.Themes copied to clipboard

[Material][TextBox] Add support for Leading/Trailing Icon/IconCommand

Open kazo0 opened this issue 2 years ago • 24 comments

specs to come, placeholder issue for now.

Basically, we would want some sort of TextBox control that can have some sort of iconbutton that will fire a command when clicked. Picture a Search Box where you can type into it and either Enter or tapping on the magnifying glass icon will fire the command.

Unclear at this point if this should be in Themes or Toolkit

kazo0 avatar Aug 31 '22 18:08 kazo0

I was brainstorming on what would be the best approach to implement the TextBox with a trailing icon + command and doing some tests. @NVLudwig / @kazo0 by default the uwp/winui TextBox includes a delete button that appears when text is entered in the box. When a user clicks on it, the text in the TextBox is cleared. I don't think of the scenario where we will have a trailing icon + command (like a search icon) in addition/next to the default delete button with the default clear all action right ?

I was thinking @kazo0 that by default it will be the default delete button (*except for specific cases described below), but if a control extensions or custom properties are added we can maybe collapse the default delete button (that depends on specific default code-behind of the TextBox) and display the other button with the icon and command that we need.

  • IF we go with a new TextBox Styles approach with new extensions to accommodate the additional icon and command I did a little test on this branch (dev/agzi/TestIconButtonTextBox-Styles) with the TextBox Filled Style, but basically, we could have something like this at the end I think:
<!-- Better name for the new Style needs to be brainstormed -->
                        <TextBox PlaceholderText="Filled TextBox with a Trailing Icon and Command"
                                 um:ControlExtensions.Command="{Binding SearchContact}"
                                 Style="{StaticResource MaterialFilledWithTrailingIconTextBoxStyle}">
                            <um:ControlExtensions.LeadingIcon>
                                <SymbolIcon Symbol="Contact" />
                            </um:ControlExtensions.LeadingIcon>
                            <um:ControlExtensions.TrailingIcon>
                                <SymbolIcon Symbol="Find" />
                            </um:ControlExtensions.TrailingIcon>
                        </TextBox>

But we will have 4 Styles in total at the end (with the two new styles):

    • MaterialFilledTextBoxStyle Filled style that has the default delete button behavior
    • MaterialFilledWithTrailingIconTextBoxStyle (better name to be found) Filled style that has the extensions for the leading/trailing icon + command extensions but without the default delete button behavior
    • MaterialOutlinedTextBoxStyle Outlined style that has the default delete button behavior
    • MaterialOutlinedWithTrailingIconTextBoxStyle (better name to be found) Outlined style that has the extensions for the leading/trailing icon + command extensions but without the default delete button behavior
  • IF we go with another new custom control based on TextBox to be able to add the icon and command properties I'm wondering about the necessity of creating a control that facades the button properties for the trailing icon and command, but if we go that route maybe we can better manage with the code behind and have only one custom control based on TextBox that can have the default delete button behavior when no trailing icon/command are specified and in the opposite situation the button with the trailing icon and command will be available and the delete button collapsed. That way we can continue to only keep 2 styles (MaterialFilledTextBoxStyle and MaterialOutlinedTextBoxStyle), but we will not use the default TextBox control, just the new custom one based on it.

@kazo0 let me know if you think of other possibilities that I didn't think about it and what approach you will like the best in your opinion.


*Specific cases to take into consideration: Note also that by default with the uwp/winui TextBox, the delete button is shown only for editable, single-line text boxes that contain text and have focus. The delete button is not shown in any of these cases:

  • IsReadOnly is true (not editable)
  • AcceptsReturn is true (multiline textbox)
  • TextWrapping is Wrap (text is wrapping inside the textbox)

agneszitte avatar Sep 12 '22 13:09 agneszitte

@Xiaoy312 suggestions:

we probalby dont need to define a new control, just reuse the existing one and just alter the base material style to support them, or make an icon-supporting copy

the contract would look something like:

<TextBox Text=...
    utu:InputExt.LeadingIcon=...
    utu:InputExt.LeadingIconCommand=...
    utu:InputExt.TrailingIcon=...
    utu:InputExt.TrailingIconCommand=... />

"visual":

outlined: [{leading-icon}{text/placeholder}{X-delete}{trailing-icon}]
filled:
+--------------+                               +---------------+
|{leading-icon}|{text/placeholder}   {X-delete}|{trailing-icon}|
+==============+===============================+===============+

IsReadOnly or TextWrapping shouldnt be of concern if we are leveraging TextBox; as for multiline, so long as we v-center/bottom (design choice) the icons, it wouldnt be a problem either

optionally we can throw in another attached property to control if x-clear/delete should ever appear at all

agneszitte avatar Sep 12 '22 13:09 agneszitte

Am ok with a new IconButtonTextBoxStyle approach. Let's start with Outlined and do Filled later once it works.

Not sure if there's the need to have both the new Button AND the Delete button present at the same time.

It would use ControlExtensions.TrailingIcon and ControlExtensions.Command Attached Properties.

francoistanguay avatar Sep 12 '22 13:09 francoistanguay

@kazo0 not sure if this helps, but it sounds like the functionality you described above is available in: AutoSuggestBox Class which seems a lot like Search in Material

NVLudwig avatar Sep 12 '22 14:09 NVLudwig

@kazo0 not sure if this helps, but it sounds like the functionality you described above is available in: AutoSuggestBox Class which seems a lot like Search in Material

yes @NVLudwig but in this case, the implementation needs to be more general, not just for searching purposes in order to be able to have any trailing icon we want for the TextBox with any command related to this icon

agneszitte avatar Sep 12 '22 14:09 agneszitte

  1. MaterialFilledTextBoxStyle
  2. MaterialFilledIconButtonTextBoxStyle
  3. MaterialOutlinedTextBoxStyle
  4. MaterialOutlinedIconButtonTextBoxStyle

We are going with these two new names for now after I synced with @kazo0 and @Xiaoy312 Can be changed later on if needed before doing final documentation and tests

agneszitte avatar Sep 12 '22 15:09 agneszitte

@NVLudwig do you think of a scenario or the need to have the trailing icon button AND the Delete button present at the same time?

agneszitte avatar Sep 12 '22 15:09 agneszitte

@agneszitte-nventive The Search Scenario requires both the clear and an extra button to confirm the operation Screen Shot 2022-09-12 at 10 42 09 AM

Error Handling could require an icon on the right (not actionable) instead of the clear button image

NVLudwig avatar Sep 12 '22 15:09 NVLudwig

@agneszitte-nventive The Search Scenario requires both the clear and an extra button to confirm the operation Screen Shot 2022-09-12 at 10 42 09 AM

Error Handling could require an icon on the right (not actionable) instead of the clear button image

cc @francoistanguay, @kazo0

agneszitte avatar Sep 12 '22 16:09 agneszitte

Yeah I'd say the delete button should be available on its own since it is built into the TextBox control itself. We wouldn't need to use the attached properties and a command for the delete. I think having both is fine, there's no exposed property to hide the clear button though I don't think.

kazo0 avatar Sep 12 '22 16:09 kazo0

Hum then let me think about a way of having only one style for Outlined (same with only one style for Filled later on). So adjusting the current MaterialOutlinedTextBoxStyle to manage the wanted behavior:

  • Adding the trailing icon using ControlExtensions.TrailingIcon and ControlExtensions.Command Attached Properties
  • Updating ControlExtensions.Icon to ControlExtensions.LeadingIcon Attached property so it's more clearer for the leading icon
  • Finding a way to hide the delete button when it's not needed (maybe with another new attached property? I will do some tests)
  • Error Handling will be done with the ControlExtensions.TrailingIcon visible to show the error icon, no command (as not actionable), and hiding the delete button maybe? But @NVLudwig is there a scenario where we will have the trailing icon + delete button and when there is an error the trailing icon will remain and only the delete button will become the error icon not actionable?

cc @kazo0, @Xiaoy312, @francoistanguay

agneszitte avatar Sep 12 '22 17:09 agneszitte

But @NVLudwig is there a scenario where we will have the trailing icon + delete button and when there is an error the trailing icon will remain and only the delete button will become the error icon not actionable?

I would imagine that is an invalid scenario. IF The clear button only exists when there is an input. IF the input is proven invalid the error badge should prevent the user from launching the same request again, THEN the error badge should replace the action button OR it should replace both the input and clear buttons.

So the possible error scenario are error badge + clear button. OR error badge alone and user has to manually clear input (this seems to be the case in the Auto complete and Search Component for WinUI and Material)

NVLudwig avatar Sep 12 '22 18:09 NVLudwig

But @NVLudwig is there a scenario where we will have the trailing icon + delete button and when there is an error the trailing icon will remain and only the delete button will become the error icon not actionable?

I would imagine that is an invalid scenario. IF The clear button only exists when there is an input. IF the input is proven invalid the error badge should prevent the user from launching the same request again, THEN the error badge should replace the action button OR it should replace both the input and clear buttons.

So the possible error scenario are error badge + clear button. OR error badge alone and user has to manually clear input (this seems to be the case in the Auto complete and Search Component for WinUI and Material)

Thanks for the feedback, make sense to me also

agneszitte avatar Sep 12 '22 19:09 agneszitte

we will need 3 icons:

  • Leading Icon,Command,Is{0}Visible + {0}}Command: customizable via attached-dp
  • delete button: not-customizable & hidden from developer ^ you will manage this with visual-states:"ButtonVisible|ButtonCollapsed" (https://github.com/unoplatform/uno/blob/master/src/Uno.UI/UI/Xaml/Controls/TextBox/TextBox.cs#L865-L881) ^ the binding is automagically done with x:Name="DeleteButton"
  • Trailing Icon,Command,Is{0}Visible: customizable via attached-dp

trailing and delete button are 2 separate concepts in the context of "error badge + clear button", it fits into the above setup as "TrailingIcon + delete button" we can have them both coexisting, or make error badge to go away by manipulating IsTrailingIconVisible based on focus

GitHub
Build Mobile, Desktop and WebAssembly apps with C# and XAML. Today. Open source and professionally supported. - uno/TextBox.cs at master · unoplatform/uno

Xiaoy312 avatar Sep 16 '22 14:09 Xiaoy312

@kazo0 I will sync with you about this item before moving forward

agneszitte avatar Jan 26 '23 22:01 agneszitte

Okay, @agneszitte / @Xiaoy312, let's decide on an official spec for this

Leading Icon / Command

Update ControlExtensions.Icon to ControlExtensions.LeadingIcon and add a ControlExtensions.LeadingIconCommand

For trailing icons, I believe we can have more than one icon if we want, right @NVLudwig / @SMarengere ? Or do we just want to say that we can only have one customizable trailing icon plus the already built-in delete icon button?

Trailing Icon / Command

Create ControlExtensions.TrailingIcon and ControlExtensions.TrailingIconCommand

Could we maybe support having a collection of Trailing Icons? like ControlExtensions.TrailingIcons and some sort of way of setting an icon and a command for each one?

kazo0 avatar May 19 '23 17:05 kazo0

Could we maybe support having a collection of Trailing Icons? like ControlExtensions.TrailingIcons and some sort of way of setting an icon and a command for each one?

it would have to be a DependencyObjectCollection<T> where T contains all the properties (icon, command, is-enabled) the problem is the built-in textbox's clear button wont be able to fit into this collection as that would be owned+managed by the textbox, while this collection would be the our hand

Xiaoy312 avatar May 19 '23 17:05 Xiaoy312

i don't want to touch the clear button at all, that's its own thing that is owned by TextBox. Let's just add the Command for the leading icon and a new trailing icon and trailing icon command.

Do we need the IsVisible? can people just set the Icon when they want an icon and not set it when they don't need it?

kazo0 avatar May 19 '23 17:05 kazo0

that wont be bindable, without a custom converter IsVisible is much needed

Xiaoy312 avatar May 19 '23 17:05 Xiaoy312

(In case some details here can help microsoft-ui-xaml - Proposal: Prefix and Suffix Properties for TextBox)

agneszitte avatar May 19 '23 21:05 agneszitte

So for now we will have:

Leading Icon / Command

Update ControlExtensions.Icon to ControlExtensions.LeadingIcon and add a ControlExtensions.LeadingIconCommand and ControlExtensions.IsLeadingIconVisible

Trailing Icon / Command

Create ControlExtensions.TrailingIcon and ControlExtensions.TrailingIconCommand and ControlExtensions.IsTrailingIconVisible

kazo0 avatar May 24 '23 12:05 kazo0

@kazo0 Should this issue be moved to Themes repo?

eriklimakc avatar May 25 '23 14:05 eriklimakc

@carldebilly @iurycarlos can you add some QA time for this in the plugin project please.

NVLudwig avatar Nov 23 '23 15:11 NVLudwig

Currently blocked by: https://github.com/unoplatform/Uno.Themes/issues/1286

On Windows (WinUI - only) we cannot use Leading/Trailing Icon (+ Command & Visibility). Even if we add only a TextBox without the new DPs the app will crash with an Unhandled exception that doesn't give much detail.

If we try using LeadingIcon we get:

image

If we replace (on the template) um:ControlExtensions.LeadingIcon for um:ControlExtensions.Icon it works. Obs: LeadingIcon and TrailingIcon are basically a copy of Icon.

I have also tried using Content="{TemplateBinding um:ControlExtensions.LeadingIcon}" as suggested here, but it didn't work either.

eriklimakc avatar Dec 06 '23 10:12 eriklimakc