✨ improved the UX for the right sidebar
Description
As a user when I: - Click the hamburger menu it makes the difference when it is open and when it is closed .☰ for open and X for close . - Click somewhere else in the page the right side bar closes. - Press the esc button the right sidebar collapses.
This PR fixes #50
Checklist
- [x] My code follows the style guidelines of this project
- [x] I have performed a self-review of my code
- [ ] My changes generate no new warnings
##Screenshots
Hey, thanks for taking the time to submit another PR.
I reviewed it locally and found a few things that would need to be addressed before merging:
-
It's behind
main -
It's triggering the
ResizeObserverLooperror when devtools is open. This used only to happen occasionally, but now it happens whenever collapsing the menu. AFAIK, it's only an issue in dev, but seeing that runtime error hurts the developer experience, which I worry will dissuade others. -
Opening and collapsing the sidebar triggers API requests.
Having the X icon is a better UI experience, but I'm afraid the extra issues this PR introduces aren't worth that benefit. If they're resolved, I'd be happy to do another review
~~ Its collapsing behavior should follow the same pattern as the left sidebar. Specifically, when the sidebar is open and a user clicks on the all-day grid, main grid, or left sidebar, then the right sidebar should simply collapse.~~ ~~Currently, the right sidebar collapses and the regular click action is created (starting a new event), which can be annoying if a user just wants to collapse the right sidebar~~
- Removed this required, because it's already doing that in
main. This can be addressed in a separate PR.
Hey, no worried. Let me look into it 👍
Hey Tyler, I have been looking into it. As you said earlier opening and closing the navbar calls a API request tell me more about it. Furthermore, I would like to know more about the ResizeObservable.
What's in my mind is that when the X button changes to ☰ , it causes a change in the size of DOM element which now occurs without letting the observer function know. I think all along this is why maybe you chose to let the ☰ stay in both open and close state. I feel like its a small logical error that I can fix but am struggling to identify it.
I'll appreciate if you could help me figure out what could be a possible solution to fix the undelivered notification. Thank you
As you said earlier opening and closing the navbar calls a API request tell me more about it.
If you watch the backend logs, you'll see that opening and closing the sidebar triggers event requests. They'll look something like this:
200 GET /api/event?start=2024-03-03T00:00:00-06:00&end=2024-03-09T23:59:59-06:00 39.615ms Sat, 09 Mar 2024 23:33:34 GMT
200 GET /api/event?someday=true&start=2024-03-01&end=2024-03-31 43.044ms Sat, 09 Mar 2024 23:33:34 GMT
200 GET /api/event?someday=true&start=2024-03-01&end=2024-03-31 41.308ms Sat, 09 Mar 2024 23:33:36 GMT
Why that happens gets into how state is being accessed and updated throughout the app, along with the effect hooks you've created on this branch. I wish I could give a quick explainer, but it'd take a lot of debugging to confirm what exactly is triggering this behavior. It's potentially caused by poorly architected state management, your implementation of the effect hooks, or both.
Since this re-rendering and subsequent API requests will to slow down the UX, I'm afraid we can't proceed without fixing the underlying issues first.
I didn't get the chance to dig into the ResizeObservable error, but the above issue is already a show-stopper, so it's not worth worrying about that at this point.
Converted to draft due to the issues mentioned above