Fix #4844: 🔨 Retain the open state of the calendar popup on document visibility change

[balajis-qb]

Closes #4844

Description

Retain the open state of the calendar popup on document visibility change.

Problem
As mentioned in the issue description, the calendar gets auto-reopen whenever we switch the browser tab and switch back to the original tab in any of the below 2 cases

1. Switching the browser tab after we enter the date manually into the calendar input by dismissing the calendar popup
2. Switching the tab immediately after selecting any date via calendar popup (In this case, we could have blurred focus from the input, but when I inspect the code, we intentionally refocus the calendar input, upon selection of any date from the calendar popup as a fix for the issue Calendar pop-up window does not close if using Macbook touchpad #628)

In any of the above cases the input is focused while switching the browser tab and when we switch back to the original tab, the browser refocus the calendar input, which will trigger our handleFocus handler in our code and we open the calendar popup regardless of it's previous open state

Changes
As a fix, I listened for the visibilitychange event and note whenever the tab gots hidden. Whenever the input gets refocused, if it gets auto re-focused because of the browser visibilitychange, I used the previous open state to decide whether to re-open the calendar popup or not.

• This fix helps us to keep the calendar open, when it was opened before the browser tab switch
• This fix also helps us to avoid re-open the calendar popup unnecessarily when it doesn't get opened before the browser tab switch

Basically the solution I suggested helps to retain the previous open state of the calendar popup whenever the browser gets re-opened.

Contribution checklist

• I have followed the contributing guidelines.
• I have added sufficient test coverage for my changes.
• I have formatted my code with Prettier and checked for linting issues with ESLint for code readability.

[pullrequest]

✅ This pull request was sent to the PullRequest network for review. Expert reviewers are now being matched to your request based on the code's requirements. Stay tuned!

What to expect from this code review:

• Comments posted to any areas of potential concern or improvement.
• Detailed feedback or actions needed to resolve issues that are found.
• Turnaround times vary, but we aim to be swift.

---

@balajis-qb you can click here to see the review status or cancel the code review job.

[pullrequest]

PullRequest Breakdown

Reviewable lines of change

+ 91
- 1

55% TSX (tests)
45% TSX

Type of change

Fix - These changes are likely to be fixing a bug or issue.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.73%. Comparing base (8e7dbce) to head (59a42e3).

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #4878      +/-   ##
==========================================
+ Coverage   96.72%   96.73%   +0.01%     
==========================================
  Files          28       28              
  Lines        2990     3004      +14     
  Branches     1293     1293              
==========================================
+ Hits         2892     2906      +14     
  Misses         98       98              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pullrequest]

The updates here look good and are aligned with the description of the problem and change. As long as the testing of retrieval from the document are working as expected this appears to be good to go.

Logan A

---

Reviewed with ❤️ by PullRequest

[pullrequest]

The changes look good overall, however I added a comment about a non-null assertion that may be preferable to handle without that assertion for safer coding practices.

Jonah

---

Reviewed with ❤️ by PullRequest

[pullrequest]

ISSUE: @typescript-eslint/no-non-null-assertion (Severity: Low)
Forbidden non-null assertion.

Remediation:
The suggestion is to change the function to not rely on the non-null assertion:

it("should retain the calendar open status when the document visibility changes", () => {
  const { container } = render(<DatePicker />);
  const input = container.querySelector("input");
  if (input) {
    fireEvent.click(input);
    expect(container.querySelector(".react-datepicker")).toBeTruthy();
    jest.spyOn(document, "visibilityState", "get").mockReturnValue("hidden");
    fireEvent(document, new Event("visibilitychange"));
    jest.spyOn(document, "visibilityState", "get").mockReturnValue("visible");
    fireEvent(document, new Event("visibilitychange"));
    expect(container.querySelector(".react-datepicker")).toBeTruthy();
  } else {
    throw new Error("Input element not found");
  }
});

🤖 powered by PullRequest Automation 👋 verified by Jonah

[balajis-qb]

Thank you Jonah for your review. I updated the code as per your suggestion.

[pullrequest]

PullRequest reviewed the updates made to #4878 up until the latest commit (59a42e3). No further issues were found.

Reviewed by:

Jonah