Skip to content

Fix like icon position(vertical position) on search page#603

Open
matsujun wants to merge 1 commit into
masterfrom
fix_like_icon_position_on_search_page
Open

Fix like icon position(vertical position) on search page#603
matsujun wants to merge 1 commit into
masterfrom
fix_like_icon_position_on_search_page

Conversation

@matsujun

@matsujun matsujun commented Feb 4, 2018

Copy link
Copy Markdown

Overview (Required)

Sorry... my previous PR had one lack of consideration...
If the session has 2 or 3 speakers, like icon moves up.
It's better to place like icon at same place on every session card.
So I changed to align it to most bottom speaker vertically in this PR.

// Each speaker icon size is 28dp.
// it has 4dp margins
// and speakers area has 16dp top/bottom mergines
// 28 + 4 * 2 + 16 * 2 = 68dp

Screenshot

Before After

Changed to align most bottom speaker vertically.

Each speaker icon size is 28dp.
it has 4dp margins
and speakers area has 16dp top/bottom mergines
28 + 4*2 + 16*2 = 68dp

<io.github.droidkaigi.confsched2018.presentation.common.view.FavoriteButton
android:id="@+id/favorite"
android:layout_height="68dp"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a question 🙋
Why is this height needed? 🤔

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

If I don't set height, it's wrap_content.
In that case, even if we have only one speaker, the height of favorite is lower than speakers .
So if we don't set this height, the vertical position of favorite and name of speakers are not same.

I think it looks not good, the vertical position of most bottom speaker and like button seems better to be same.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can we use constraintbottom_bottomOf="speaker" for favorite?

@matsujun

matsujun commented Feb 4, 2018

Copy link
Copy Markdown
Author

Ahhhh... This change looks not needed...
Current (before this PR) "like" layout is same as normal (not search) session list.
I close this PR.

@takahirom

Copy link
Copy Markdown
Member

If you have time, I think it would be better to fix the session list together 👀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants