Popup in compact interactive list #2149
Conversation
… popup-in-compact-interactive-list-cell
| @@ -237,6 +237,8 @@ class PopupContent extends React.Component { | |||
| onResize={this.handleOnResize} | |||
| refCallback={refCallback} | |||
| role={popupContentRole || null} | |||
| onKeyDown={event => event.stopPropagation()} // Added for Popup support in terra-compact-interactive-list. As focus trap doesn't stop key press event propagation to the CIL cell, it interferes with cell key press event handler. | |||
There was a problem hiding this comment.
You should create a function will a useCallback hook for memoization to prevent unnecessary rerenders. Same comment for your new onFocus function.
There was a problem hiding this comment.
Fixed here: 0dc66ba
| @@ -237,6 +237,8 @@ class PopupContent extends React.Component { | |||
| onResize={this.handleOnResize} | |||
| refCallback={refCallback} | |||
| role={popupContentRole || null} | |||
| onKeyDown={event => event.stopPropagation()} // Added for Popup support in terra-compact-interactive-list. As focus trap doesn't stop key press event propagation to the CIL cell, it interferes with cell key press event handler. | |||
There was a problem hiding this comment.
Do you want to stop the propagation for all keyboard actions or specific ones like Tab. I am not sure we want to blindly stop all key event propagation.
There was a problem hiding this comment.
The keys that might mess with the CIL are Tab and Arrow keys (Right/Left/Up/Down). However, as the key event propagation has been stopped on the popup content wrapper <div>, it only stops bubbling up to the CIL. The key event for all keys (including TAB) will still work inside the popup the same way they did before.
| buttonRef.current.focus(); | ||
| }; | ||
|
|
||
| const handleOnChange = (e) => { |
There was a problem hiding this comment.
This should have a useCallback hook
There was a problem hiding this comment.
Callback added: cacc605
| const handleButtonClick = () => { setIsOpen(true); }; | ||
| const handleRequestClose = () => { setIsOpen(false); }; |
There was a problem hiding this comment.
These should have a useCallback hook.
There was a problem hiding this comment.
Callback added: cacc605
Co-authored-by: Saad Adnan <[email protected]>
Summary
Terra compact interactive list has to support terra-popup in its cell. As compact interactive list has its own key and focus event handlers, terra-popup content key and focus events propagation needs to be stopped in order for compact interactive list to preserve key navigation between cells.
This PR:
Testing
This change was tested using:
Reviews
In addition to engineering reviews, this PR needs:
This PR resolves:
UXPLATFORM-10389