WRQ-15921: Fix popup to not focus the element of the popup container when popup is hidden

[juwonjeong]

Checklist

• [x] I have read and understand the contribution guide
• [ ] A CHANGELOG entry is included
• [x] At least one test case is included for this feature or bug fix
• [x] Documentation was added or is not needed
• [ ] This is an API breaking change

Issue Resolved / Feature Added

In FixedPopupPanels when popup is closed, focus does not restore into the background component. It restore focus to the element in the popup panels.

Resolution

Disables the popup container selector when the popup is hidden. It will filter element of the popup container when finding the next focus element while restoring the focus.

Additional Considerations

Links

WRQ-15921

Comments

[codecov[bot]]

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 81.61%. Comparing base (c9c4d77) to head (e13037c).

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1604      +/-   ##
===========================================
+ Coverage    81.59%   81.61%   +0.01%     
===========================================
  Files          148      148              
  Lines         6673     6680       +7     
  Branches      1986     1989       +3     
===========================================
+ Hits          5445     5452       +7     
  Misses         935      935              
  Partials       293      293              

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

[0x64]

The bug occurs when activator is not correctly set. So, I prefer to show up the issue condition directly. I'm not sure that what we're supposed to do when current does not exist, but it looks okay to handle this case with the same manner. My idea looks like the below. (Just idea, not tested fully.)

  if (!current || (containerNode && containerNode.contains(current))) {
  	// attempt to set focus to the activator, if available
  	if (!Spotlight.isPaused()) {
  		if (activator) {
  			if (!Spotlight.focus(activator)) {
  				Spotlight.focus();
  			}
  		} else {
  			Spotlight.disableSelector(lastContainerId);
  			Spotlight.focus();
  			Spotlight.enableSelector(lastContainerId);
  		}
  	}
  }

[juwonjeong]

The bug occurs when activator is not correctly set. So, I prefer to show up the issue condition directly. I'm not sure that what we're supposed to do when current does not exist, but it looks okay to handle this case with the same manner. My idea looks like the below. (Just idea, not tested fully.)

  if (!current || (containerNode && containerNode.contains(current))) {
  	// attempt to set focus to the activator, if available
  	if (!Spotlight.isPaused()) {
  		if (activator) {
  			if (!Spotlight.focus(activator)) {
  				Spotlight.focus();
  			}
  		} else {
  			Spotlight.disableSelector(lastContainerId);
  			Spotlight.focus();
  			Spotlight.enableSelector(lastContainerId);
  		}
  	}
  }

Thank you for the review. I agree with your comment that activator should be used as a condition. I tested it and it seems to work fine.