Fix: navbar should close when user click anywhere on screen

[lalitkumawat1m]

What kind of change does this PR introduce?

Issue Number:

• Closes ✨ Enhancement: Top Navbar should close when user clicks anywhere on the screen  #536

Screenshots/videos:

JSON.Schema.-.Personal.-.Microsoft.Edge.2024-03-20.18-07-05.mp4

If relevant, did you update the documentation?
No

Summary

Does this PR introduce a breaking change?
No

[DarhkVoyd]

Hey, I am unsure about using document.addEventListener and directly interacting with the DOM, I suggest we probably use some react event handler such as onBlur instead.

Normally you shouldn’t need to do this with React: you need to manually locate the elements using refs, which kinda breaks how React should work.

[lalitkumawat1m]

Hey @DarhkVoyd, I appreciate your suggestion but Could you please guide me how this can be done without manipulating dom and using onblur?

[DarhkVoyd]

Hey @lalitkumawat1m So, after thinking more about it, I think that your approach is the better way. I was earlier thinking about making the navbar focusable and then using onBlur listener to handle blur cases but I realised it would be unnecessary. I understand now that the use of document.addEventListener is valid in this case.

[DarhkVoyd]

The one thing I am still confused about is the use of showMobileNav in useEffect dependencies array. Wouldn't mobileNavRef be a more valid dependency instead of the showMobileNav.

[benjagm]

Hi Lalit. Can you please check the comments done by DV?

[lalitkumawat1m]

Hey @benjagm @DarhkVoyd I have made the requested changes. Please have a look

[DarhkVoyd]

Looks mostly good. just some suggestions,

1. The issue proposed to close navbar when user clicks anywhere outside the navbar. Now, the implementation and PR are focused on closing the navbar if user clicks anywhere, even on the navbar. But, still the function is named handleClickOutside, maintaining consistency would help keep things clear. Also, is this intentional?
2. A general practice suggestion, commit messages descriptive of changes introduced in code are better. This enhances the readability of the project's history. For example, instead of Made requested changes, a commit message like Update useEffect dependency from showMobileNav to mobileNavRef provides a clearer note of the change.

[lalitkumawat1m]

Hey @DarhkVoyd,

1. Should I rename the handleClickOutside function to handleCloseNavbar ?

2. I will update my commit msg to meaningfully commit msg.

[DarhkVoyd]

Sure

[lalitkumawat1m]

I have made the changes.

[benjagm]

Looks good. Thanks for fixing this!

[DarhkVoyd]

Looks good to me, thanks!