[iOS] Enable Smoother Transitions from SideMenu to Other Views

[TakayaShirai]

Issue Number

https://github.com/okuda-seminar/Twitter-Clone/issues/409

Implementation Summary

This PR enables smoother transitions from the SideMenuView to the other views, such as the view of UserProfileViewController. Previously, the entire AppRootViewController was dismissed, which was not the intended behavior. The new implementation allows the SideMenuView to be dismissed when users tap the buttons in it.

Scope of Impact

When users tap the buttons in the SideMenuView, the transitions occur and the corresponding view will appear.

Particular points to check

Please verify if the implementation of the dismissal mechanism for the SideMenuView is appropriate.

References

https://github.com/user-attachments/assets/b334c365-81f2-40d0-a93b-ec7a3d13f92d

Design Document

Although this is different from the original usage of design documents, I've created a Design Document for the past issue https://github.com/okuda-seminar/Twitter-Clone/issues/400 to consider the view hierarchy or the view controller's responsibilities more deeply. I would appreciate it if you could review this document.

Schedule

Until 10/05.

Note

I have updated the schedule from 'Until 9/28' to 'Until 10/05' as of 9/26.

[ryuji0123]

@TakayaShirai You shouldn't work on this before showing your design and don't make me point out the same thing repeatedly.

[TakayaShirai]

First, I apologize for your taking extra time and the confusion caused.

I cannot reproduce the demo locally.

I'm sorry about this. This is probably because I recently cloned the Twitter-Clone project on my local laptop again to fix errors that were caused when I accidentally moved the directory to another place. I will ask @a-shigemichi if he can reproduce the demo in his local environment as well.

As I pointed out before, why don't you write a design doc to show the responsibility of each view / view controller?

I apologize if my writing was unclear but I wrote the design doc about enabling the display of the side menu view this time to better understand the view hierarchy and each viewController's responsibilities, and I've included the link to it in this PR ( under the Design Document section).

Current one is still very weird and I'm having difficulty in understanding why you're suggesting this.

Our aim here is to only hide the SideMenu so my initial thought was that dismissing the SideMenu in itself would be fine because SideMenu was not controlled by either the ViewControllerWithUserIconButton or AppRootViewController and that operation only dismisses the SideMenu. I'm not quite sure what point is still very weird but, is it that I'm dismissing the View instead of the viewController(UIHostingController)? It's embarrassing that it seems I'm not still able to understand the view hierarchy and each viewController's responsibility correctly even after writing the design doc, I would really appreciate it if you could what point is still very weird. If the point I mentioned above is weird, I would propose adding viewWillDissapear in the ViewControllerWithUserIconButton shown below instead of the changes I proposed before.

  override func viewWillDisappear(_ animated: Bool) {
    super.viewWillDisappear(animated)
    sideMenuHostingController.dismiss(animated: true)
  }

If further discussion is needed, would it be possible to go over this issue on Saturday so as not to take too much of your time? I appreciate your understanding.

[ryuji0123]

Your current implementation is similar to what I showed in previous reviews:

1. Call present of selected view controller
2. Call dismiss of side menu

What happens if we reverse the above order? In case your code works only with the order, then the code is too sensitive to it and this is very weird. Also, since we're using DrawerTransitionController, we have to make the controller responsible for both presenting and dismissing transitions. If we call dismiss from SideMenuView directly, then it means users must know how DrawerTransitionController presents views and if that logic is changed in the future due to some bugs or issues, then users must change all use cases. This is clearly bad-practice.

As for VC's responsibility, even with the current view hierarchy, as AppRootViewController has pointers to both ViewControllerWithUserIconButton and SideMenuView, it is totally making sense to have AppRootViewController still manage which view will be presented. However, your code splits the role into several parts like:

1. To present VC inheriting from ViewControllerWithUserIconButton class -> AppRootViewController
2. To present side menu -> VC inheriting from ViewControllerWithUserIconButton class
3. To dismiss side menu -> SideMenuView

I cannot believe this is making sense. ViewControllerWithUserIconButton is just wrapping SideMenuView and it mainly aims to work as SideMenuViewController in your doc, then why is VC itself managing the transition for Side Menu? Transition of Side Menu includes that of ViewControllerWithUserIconButton, so why do you think VC other than AppRootViewController should take the responsibility?

[a-shigemichi]

I tried a local demo during my own review, and it works as described in the References. I've recorded videos of the demo both before and after implementation, so please check them.

https://github.com/user-attachments/assets/dfaeb2e4-33df-404e-82f2-b3041da2abb7

https://github.com/user-attachments/assets/e219fdbe-bea5-416e-850e-a13f5a6cf9e3

[ryuji0123]

Hmm, I checked out to feature/#409_enable_smoother_transitions_from_sideMenu_to_other_views after pulling but it's not working. Anyway, I cannot approve this PR since the design isn't making sense to me as I've commented.

[TakayaShirai]

Thank you so much for your detailed and clear feedback. It has helped me gain a broader perspective on the view hierarchy, which I previously lacked. I realized I solely focused on understanding the current structure without considering the potential issues if left unchanged. Moving forward, I will restructure the view hierarchy to ensure DrawerTransitionController handles presenting and dismissing, while AppRootViewController will take responsibility for both ViewControllerWithUserIconButton and SideMenuView.