Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
お気に入り画面の新着タブのRecyclerViewとCoordinatorLayout実装 #20
お気に入り画面の新着タブのRecyclerViewとCoordinatorLayout実装 #20
Changes from all commits
c3bc24f
a4db18f
f6f4786
fa095a0
05c60ed
e9ebee1
24f01fd
00303bd
d082054
7493d3d
78db7bc
6597b49
e63e0ea
734af97
48f3363
931691d
bf47b07
d71ff43
7cf2ab4
4075645
b47e41a
80eeef9
102c9c7
1ecb962
1d7b4bf
76c399e
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
せっかくなのでAPIから返ってきた想定でデータはこっちで持っておきましょうか
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
あと多分お気に入りのブランドがあるかないかで表示が結構変わるのでそこも実装してもらえると 🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
APIからでーた持ってきた前提に修正しました!!
お気に入りのブランドがあるところはまた長くなりそうなので別のプルリクであげます!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
MaterialDesign的には左右16dpのmarginなんですが、16dpだと大きかったですか?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
純粋に見逃してました!🙏
コンポーネントに直接marginつけるのよくないかもと思い直したので、FavoriteItemCell内部でbindのタイミングで動的に左右に16dpのmarginをセットするようにしました!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
お?これはどんな理由があります?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
純粋に同じレイアウト再利用するときに画面によってマージンが変わるかもしれないからです!ただ今回はそこまで入らない気がしますが笑
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
基本的には左右16dpのマージンは変わらないのと、もしマージンが違うデザインだった場合、もはやそれは違うitemなんじゃないかなとは思いますね
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
確かにそれもそうですね!修正しときます!ありがとうございます🙏
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
startなしで、endの制約を
guideline_vertical2
にすればこのImageViewのmarginはいらないのでは?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
コンポーネントに直接marginつけるのよくないかもと思い直したので、FavoriteItemCell内部でbindのタイミングで動的に左右に16dpのmarginをセットするようにしました!