feat(ui5-select): Add support for disabled select options #2710
Conversation
Allow the application specify select options as disabled. Disabled options are non-interactable.
Allow the application specify select options as disabled. Disabled options are non-interactable.
…omponents into localSelectOptions
Allow the application specify select options as disabled. Disabled options are non-interactable.
…omponents into localSelectOptions
Allow the application specify select options as disabled. Disabled options are non-interactable.
Allow the application specify select options as disabled. Disabled options are non-interactable.
ilhan007
changed the title
packages/main/src/ListItemBase.js
Outdated
| @@ -75,6 +87,12 @@ class ListItemBase extends UI5Element { | |||
| return styles; | |||
| } | |||
|
|
|||
| onBeforeRendering() { | |||
| if (this.disabled) { | |||
There was a problem hiding this comment.
Everything looks good to me, here I am thinking if it would be better to handle this disabled vs. focused state in the ListItemBase.css (for 3 or 4 selectors) in the following fashion:
:host([focused]:not([disabled]))I did not test it, but I kind of prefer it, if it is possible, instead of overriding the "focused" state.
Also, I expected the presence of tabindex="-1" to prevent getting the focus or the focusin event is still fired, or this is for cases when the item is just focused and then becomes disabled dynamically.
There was a problem hiding this comment.
Yes, this is exactly the case when a focused item is disabled.
Did you mean replacing the onBeforeRendering logic in ListItemBase.js with more precise CSS selectors?
=>
I started this way, but then noticed that the presence of the 'focused' attribute [on a disabled list-item] does not actually cause any visual issue (it has no effect anymore because a disabled list-item does not have the 'ui5-li--focusable' class) => no visual issue there => I cancelled modifying the selectors for the focused items
Unfortunately, there was still a different issue (unsolved in my previous code):
Issue:
If we disable a focused list-item, the browser does not automatically remove the focus outline from the disabled item, even after the disabled item receives tabindex=-1 it DOM
=> I was able to remove the outline only after calling blur()
=> I updated the code to call blur() in the above case onAfterRendering
I pondered really a lot which way to go.. No problem to change the solution if you have a better idea in mind! I'm around for a call for quicker clarifications if needed! Thnx!
There was a problem hiding this comment.
In my latest commit I removed the blur and made a different solution for this issue:
added some code to onAfterRendering of the Select:
In this way, if a focused list-item was disabled while the picker was still open , then onAfterRendering we ensure the focus is moved to the newly selected item
=> the browser no longer keeps the focus on the disabled item
Let me know if you have a better idea/see ways to improve on that. Or do we actually need to handle this case (of the app disabling a currently focused item) ... If this is a very edge case, I wonder if the added complexity is worth it, but I can't really judge...
There was a problem hiding this comment.
Yes, I meant replacing the onBeforeRendering logic in ListItemBase.js with more precise CSS selectors in my comment.
I will see the latest changes as soon as possible and let you know.
There was a problem hiding this comment.
Hello @kineticjs Didi,
thanks for the change, it looks good to me, I have few comments that you can find below.
ilhan007
requested review from
nnaydenow and
a team
and removed request for
nnaydenow
ilhan007
changed the title
Info and infoState attributes are added to ui5-tree-item. Fixes: SAP#2633
Follow up of SAP#2684 where the placement of the busy has been fixed. With this change we make sure the busy ind DOM is not even rendered when the List is not busy. And, aims to fix the issue with components such as the ComboBox, Select and Input, that uses the List internally, would display additional space after the last list item.
Change-Id: Iefccf8ccac86e1add8d1bcc0cc8a98ad988b2f0d
The API is deleted, since it is never used. Fixes: SAP#2605
Fixe a JS error that used to thrown upon "focusin" when non-existing DOM elements have been accessed. Those DOM elements are not rendered when there are no list items, which is the example in the referenced issue. FIXES: SAP#2717
Allow the application specify select options as disabled. Disabled options are non-interactable.
Allow the application specify select options as disabled. Disabled options are non-interactable.
Allow the application specify select options as disabled. Disabled options are non-interactable.
|
I'll fix the merge issues later and notify when ready for review |
feat(ui5-option): Add support for disabled select options
Allow the application specify select options as disabled. Disabled options are non-interactable.
FIXES: #2559