Blazorise
Blazorise copied to clipboard
Megabit
Autocomplete better keyboard handling
Prototype for #3980. Known issue - antdesign needs :focus/:hover class for anchor in the badge
https://user-images.githubusercontent.com/22550674/179865555-4bf23c04-a6be-42fa-81fd-99a569bb5251.mp4
We need to be careful here since both of you worked on a similar part of the code. @David-Moreira can you review it? If this is working as expected can and once merged can you adopt changes to your branch?
None of components using DropDown support keyboard scrolling. It would be nice to implement it at the base level somehow so that all components get this functionality
We need to be careful here since both of you worked on a similar part of the code. @David-Moreira can you review it? If this is working as expected can and once merged can you adopt changes to your branch?
I tought the same thing when I saw this... Kinda think this should have gone on top of the one that's ongoing... oh well...
None of components using DropDown support keyboard scrolling. It would be nice to implement it at the base level somehow so that all components get this functionality
Yea, You're right.
We need to be careful here since both of you worked on a similar part of the code. @David-Moreira can you review it? If this is working as expected can and once merged can you adopt changes to your branch?
I tought the same thing when I saw this... Kinda think this should have gone on top of the one that's ongoing... oh well...
Also I'm not sure I have time during the week this week. But I should be able on Saturday. Ideally we merge one of the PRs in and then handle the conflicts in the other PR.
Prototype for #3980. Known issue - antdesign needs :focus/:hover class for anchor in the badge
Bootstrap.Demo.-.Profile.1.-.Microsoft.Edge.2022-07-19.16-32-21.online-video-cutter.com.mp4
Why do you mention antdesign? Isn't it an issue in every provider? not being able to tab?
If we are supporting tabbing then It seems we should remove Tab from the ConfirmKey. And document accordingly.
What now happens is that by tabbing when selecting, it selects the item, and tabs to the next tabbable input.
Agreed? @stsrki // @glutio ?
Also it seems that after selecting I can no longer click outside the component to loose focus... It takes sometime to act, and when it does, it still shows the dropdown but with no items for 1-2s, note the little scroll arrows below the autocomplete when it finally looses focus.
EDIT: Same thing seems to happen with ESC key. After having selected something.
Why do you mention antdesign? Isn't it an issue in every provider?
I meant that ant design (at least when I used <a>) doesn’t have the visual style for focus, so when you tab to the close element you don’t see it visually that it’s focused, again did not retest it with spans. Btw, talking about Badge spec, the clickable element (Close) whether implemented as a span or as an anchor is not part of the spec, per the spec the Badge either a link as a whole or a passive display element, current implementation departs from the spec.
remove Tab from the ConfirmKey
I think so, but ideally it should match some standard behavior, if it’s possible to find any such spec. Also I think selecting an item should clear SelectedText l, I think now it stays the same which is convenient if you chose multiple items with same prefix but it’s probably not the main scenario, probably most of the type the items will not have the same prefix so after selecting one it may be better to clear the edit box so that a new prefix can be entered.
Not sure if this bug was already there, but there's a chance that it might have been... I can fix it later if you'd like
It adds both the freetyping value and the option that was click selected.
Why do you mention antdesign? Isn't it an issue in every provider?
I meant that ant design (at least when I used
<a>) doesn’t have the visual style for focus, so when you tab to the close element you don’t see it visually that it’s focused, again did not retest it with spans.remove Tab from the ConfirmKey
I think so, but ideally it should match some standard behavior, if it’s possible to find any such spec. Also I think selecting an item should clear SelectedText l, I think now it stays the same which is convenient if you chose multiple items with same prefix but it’s probably not the main scenario, probably most of the type the items will not have the same prefix so after selecting one it may be better to clear the edit box so that a new prefix can be entered.
Don't forget CloseOnSelection="false", if it's set to true, which is actually the default, it'll work as you've described.
You meant this, right?
remove Tab from the ConfirmKey
<select> closes the dropdown on tab and it cycles through items with the dropdown closed when you use arrows. maybe use <select> as the basis for behavior?
If we are supporting tabbing then It seems we should remove Tab from the ConfirmKey. And document accordingly.
Yes, Tab should be removed from confirmation keys.
remove Tab from the ConfirmKey
<select>closes the dropdown on tab and it cycles through items with the dropdown closed when you use arrows. maybe use<select>as the basis for behavior?
@stsrki
Actually I think @glutio might be right here. We should maintain the previous behaviour which should also be more inline to how other components work, like the select.
If ConfirmKey has Tab and the selection dropdown is open, then it should select the item and not TAB into something else.
However user can still Shift+Tab as it is now even if selection dropdown is open?
What do you think? Only thing, is that none of our regular Dropdowns seem to adhere to that. They stay open while you can tab all over.
Actually I think @glutio might be right here. We should maintain the previous behaviour which should also be more inline to how other components work, like the
select. IfConfirmKeyhasTaband the selection dropdown is open, then it should select the item and not TAB into something else. However user can stillShift+Tabas it is now even if selection dropdown is open?
I've just checked and you're right. select does behave like that. TBH I never noticed it before. I guess we can make it the same behavior for the Aiutocomplete.
Only thing, is that none of our regular Dropdowns seem to adhere to that. They stay open while you can tab all over.
Probably because we never considered Tab as something that triggers selection. @glutio mentioned we could add some core system that can be reused, and I can agree. But it goes far beyond this PR, so it is best to leave for another time after we finish this work. Once we settle on the right behavior, then we can make it the same for other components.
Actually I think @glutio might be right here. We should maintain the previous behaviour which should also be more inline to how other components work, like the
select. IfConfirmKeyhasTaband the selection dropdown is open, then it should select the item and not TAB into something else. However user can stillShift+Tabas it is now even if selection dropdown is open?I've just checked and you're right.
selectdoes behave like that. TBH I never noticed it before. I guess we can make it the same behavior for the Aiutocomplete.Only thing, is that none of our regular Dropdowns seem to adhere to that. They stay open while you can tab all over.
Probably because we never considered Tab as something that triggers selection. @glutio mentioned we could add some core system that can be reused, and I can agree. But it goes far beyond this PR, so it is best to leave for another time after we finish this work. Once we settle on the right behavior, then we can make it the same for other components.
Right I was thinking that also. We can do the core work later on another separate PR. Too much work on other stuff still haha
Actually I think @glutio might be right here. We should maintain the previous behaviour which should also be more inline to how other components work, like the select. If ConfirmKey has Tab and the selection dropdown is open, then it should select the item and not TAB into something else. However user can still Shift+Tab as it is now even if selection dropdown is open?
I've just checked and you're right. select does behave like that. TBH I never noticed it before. I guess we can make it the same behavior for the Aiutocomplete.
Would you make this change then, please? :)
rebased this PR on dev-1.1.0-AutoCompleteMultipleCheckbox branch and now targeting it for merge. this is meant to make it easier to merge, not sure if I succeeded here :)
it should select the item and not TAB into something else
Checkboxes receive focus on Tab, the entire DropDownItem should not receive focus
rebased this PR on dev-1.1.0-AutoCompleteMultipleCheckbox branch and now targeting it for merge. this is meant to make it easier to merge, not sure if I succeeded here :)
Yea I think this is easier now, and other PR is pratically closed.
So this still happens if you can take a look at it...
Clicking outside (loose focus) or ESC after selecting an item.
btw, about checkboxes and AutocompleteSelectionMode, do you think maybe the Mode should only be Single/Multiple and checkboxes should be a separate boolean like ShowCheckboxes ? Because Checkbox mode is also a Multuple mode, it's not really different as a mode, it's different as a view.. More practically, now I need to do (mode == Multiple || mode == Checkbox) instead of just one check for multiple mode.
btw, about checkboxes and AutocompleteSelectionMode, do you think maybe the Mode should only be Single/Multiple and checkboxes should be a separate boolean like ShowCheckboxes ? Because Checkbox mode is also a Multuple mode, it's not really different as a mode, it's different as a view.. More practically, now I need to do (mode == Multiple || mode == Checkbox) instead of just one check for multiple mode.
Already had that discussion with strski haha. We agreed upon that api.
So this still happens if you can take a look at it...
@stsrki looking at closable.js since its maybe related to popup not closing. Looking at <select> it closes its dropdown on mousedown, and closable.js uses mouseup. also it uses keyup instead of keydown for escape. Is this because of some issues when using mousedown/keydown?
Actually, closable.js works fine, it triggers all the expected events... still, just wonder about using mouseup/keyup, feel nonstandard.
Ok, the reason it's not closing, as in @David-Moreira video is OnTextBlurHandler() calls UnregisterClosableComponent(). So when we click on an item in the dropdown, the editbox loses focus and calls this handler to unregister itself but the dropdown remains open, so there's no way to close it after that. There is a number of focus issues here.
Also, for RTL support, the badges should be on the right side, how do you think to handle that? I was thinking flexbox with alignment either start or end. Is there a better way to handle RTL here? (if I may ask, do you have any actual customers who want RTL or is it just a general feature?)
About checkboxes. They accept tabs and clicks. That should be disabled, but if we use "disabled" attrubute then it will get disabled style, disabling it with tabindex=-1 and pointer-events:none feels like a hack. Should we use a real input control for checkbox here or maybe it's better to just display X (or whatever icon) and don't bother disabling input on an input control without actually properly disabling the control, we can still support theming, maybe even better, we can use different icons in different themes.... What do you think?
Changed DropDownItem @onclick to @onmousedown this prevents textbox from losing focus -https://stackoverflow.com/questions/12154954/how-to-make-element-not-lose-focus-when-button-is-pressed
I think focus/keyboard support is better now. I broke some of the behavior, so still working on it, but take a look at the changes so far in case you disagree. Thanks!
What is the expected behavior of AutoSelectFirstItem ? When should it be selected? On first render in single select mode?
Also for SetParametersAsync(), is this some kind of pattern that works? You have only two variables - propertyVal and parameterVal, what I used previously was 3 variables - propertyVal, parameterVal and oldParameterVal.. Currently there's some issue with two way binding and existing SetParametersAsync() implementation, the bound value resets the locally changed value, so I wonder if it's because this pattern is broken?
I have tried the demo with the new onmousedown on DropdownItem, and there is an issue there. It now closes as soon as you click on it. Where before, it would allow you to click, hold the button, then on release, it would close. While doing it, the item would change color to blue, and with the new implementation, that is not happening. You can also check the BS5 documentation for the same behavior https://getbootstrap.com/docs/5.2/components/dropdowns/
Also for SetParametersAsync(), is this some kind of pattern that works? You have only two variables - propertyVal and parameterVal, what I used previously was 3 variables - propertyVal, parameterVal and oldParameterVal.. Currently there's some issue with two way binding and existing SetParametersAsync() implementation, the bound value resets the locally changed value, so I wonder if it's because this pattern is broken?
Managing two-way binding is hard with SetParametersAsync, but we did it in the past. You can look for inspiration on our components like DatePicker, NumericPicker, etc. They basically check if the new param value has changed and then do the right action from there.