nishiki-tech / nishiki-frontend

Nishiki is an app for tracking and sharing food inventories within groups for better pantry management.
https://nishiki.tech
MIT License
20 stars 5 forks source link

Feat: Split the Drawer component #186

Closed nick-y-ito closed 8 months ago

nick-y-ito commented 8 months ago

Overview

This pull request introduces two new components: BaseDrawer and PrimaryDrawer. The purpose of this change is to demonstrate how to follow the Open-Closed Principle of the SOLID principles when creating multiple variations of drawer components. Please note that the existing drawer components (Drawer.tsx, DrawerUsageExample.tsx, SecondaryDrawer.tsx, and SecondaryDrawerUsageExample.tsx) have intentionally been kept for reference.

Changes

Review points

Screen Captures

Note

Assignee Checklist:

Reviewer Checklist:

nick-y-ito commented 8 months ago

@kotaaaa @kanta1207 @ShoeheyOt

Hi guys, please let me share my thoughts on how to handle this PR. As per our earlier verbal discussion, we have decided that we no longer need the PrimaryBottomDrawer. However, I think it would still be good to merge this PR, maybe along with @ShoeheyOt 's PR, to preserve the BaseDrawer and provide an example of how to implement a customized drawer for each need by inheriting BaseDrawer. We just need to make sure to delete the SecondaryDrawer when merging into the develop branch.

Therefore, I renamed PrimaryBottomDrawer to ExampleDrawer and added some comments to indicate that it's an example file.

To know how I updated after the verbal discussion, please take a look at the commits after cd19587 (#186)

I'd also love to hear your opinions.

kanta1207 commented 8 months ago

@kotaaaa @kanta1207 @ShoeheyOt

Hi guys, please let me share my thoughts on how to handle this PR. As per our earlier verbal discussion, we have decided that we no longer need the PrimaryBottomDrawer. However, I think it would still be good to merge this PR, maybe along with @ShoeheyOt 's PR, to preserve the BaseDrawer and provide an example of how to implement a customized drawer for each need by inheriting BaseDrawer. We just need to make sure to delete the SecondaryDrawer when merging into the develop branch.

Therefore, I renamed PrimaryBottomDrawer to ExampleDrawer and added some comments to indicate that it's an example file.

To know how I updated after the verbal discussion, please take a look at the commits after cd19587 (#186)

I'd also love to hear your opinions.

Thank you for sharing & summarizing your thoughts on this and I totally agree with you! I especially liked your idea on renaming PrimaryBottomDrawer to be Example, that's really a nice way to keep this work and Shu and Yusuke's effort for future!

nick-y-ito commented 8 months ago

@kanta1207 @kotaaaa @ShoeheyOt Thank you for your feedback :) I will merge it now!