Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Storyboard replacement/Home #130

Draft
wants to merge 26 commits into
base: change/storyboard_to_uikit_code
Choose a base branch
from

Conversation

shusuke0812
Copy link
Owner

@shusuke0812 shusuke0812 commented Jun 15, 2024

プルリク内容

  • 🔧 改善

やったこと

  • KikurageDataフレームワークを追加(Data層の型をUIPropsに指定する必要があったため)
  • リファクタリング
    • KikurageUI/MoleculesにKUIDropdownTextFieldを追加
    • DeviceRegisterの栽培開始日入力フィールドをKUIDropdownTextFieldに置き換え

確認したこと

補足

  • なし

@shusuke0812 shusuke0812 self-assigned this Jun 15, 2024
@shusuke0812 shusuke0812 marked this pull request as draft June 16, 2024 13:51
ASSETCATALOG_COMPILER_GENERATE_SWIFT_ASSET_SYMBOL_EXTENSIONS = YES;
CLANG_CXX_LANGUAGE_STANDARD = "gnu++20";
CLANG_ENABLE_MODULES = YES;
CLANG_WARN_QUOTED_INCLUDE_IN_FRAMEWORK_HEADER = "$(inherited)";
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

memo:

pod install errors
スクリーンショット 2024-07-13 23 21 57

solved
Yes -> No - $(inherited)
スクリーンショット 2024-07-13 23 21 36

@shusuke0812
Copy link
Owner Author

shusuke0812 commented Jul 14, 2024

マルチモジュール

レイヤードアーキテクチャ
https://martinfowler.com/bliki/PresentationDomainDataLayering.html

image

考えたこと
KUIDeviceStatusImageViewのPropsとしてKikruageDataモジュールに定義したKikurageState型を渡している. しかし、KikurageStateが変わったら(API、そのレスポンス型を変更したら)、UI層も変える必要が発生する. よって、KUIDeviceStatusImageViewのPropsとしては標準で定義されたプリミティブな型を渡した方が依存関係の複雑さを防ぐことができて保守しやすいかもしれない.

その場合は、下記のような構成にする. ただし、本プロジェクトではServiceはViewModelとする.

image

結論
KikurageDataモジュールではなく、KikurageDomainに変更する. 既にあるKikurageFeatureはKikurageDomainに統合する.(別PRで実施)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant