WRR-6380: Converted Popup/Popup to functional component #1734
Conversation
…eature/WRR-6380
…emoved unnecessary `useEffect`
…eature/WRR-6380
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1734 +/- ##
===========================================
+ Coverage 80.81% 80.83% +0.02%
===========================================
Files 148 148
Lines 6681 6695 +14
Branches 1986 1987 +1
===========================================
+ Hits 5399 5412 +13
- Misses 973 974 +1
Partials 309 309 ☔ View full report in Codecov by Sentry. |
Popup/Popup.js
Outdated
| const containerId = useRef(Spotlight.add()); | ||
| const paused = useRef(new Pause('Popup')); |
There was a problem hiding this comment.
do we need useRef or an useMemo can be used?
There was a problem hiding this comment.
useMemo can be used but useRef is also a good option because I just need to keep a reference that persists across re-renders
| const handleComponentUpdate = useCallback(() => { | ||
| if (open !== prevOpen) { | ||
| if (!noAnimation) { | ||
| if (!open && popupOpen === OpenState.OPENING || !open && popupOpen === OpenState.OPEN) { |
There was a problem hiding this comment.
can you explain why this changed from using popupOpen === OpenState.CLOSED ?
There was a problem hiding this comment.
with the current implementation and async state change of the popupOpen, if we use popupOpen === OpenState.CLOSED, the spotlight navigation will not be resumed when the popup is open and closed quickly
|
|
||
| // if there is no currently-spotted control or it is wrapped by the popup's container, we | ||
| // know it's safe to change focus | ||
| if (!current || (containerNode && containerNode.contains(current))) { | ||
| // attempt to set focus to the activator, if available | ||
| if (!Spotlight.isPaused()) { | ||
| if (!Spotlight.isPaused() || !paused.current.isPaused()) { |
There was a problem hiding this comment.
why was this additional condition needed?
There was a problem hiding this comment.
With this additional condition, the activator (component that opened the Popup) will be focused after the Popup is closed when it's used in other components like Input
Checklist
Issue Resolved / Feature Added
At this moment, Popup/Popup is based on a class component. The goal is to convert it to a functional component.
Resolution
Converted Popup/Popup to functional component
Additional Considerations
Links
WRR-6380
Comments
Enact-DCO-1.0-Signed-off-by: Ion Andrusciac [email protected]