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.

Sign up for GitHub

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

Владимир Каткалов @katkalov #25

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Владимир Каткалов @katkalov #25

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
62d5455
add https://github.com/yamblz-native/custom-viewgroup homework realis…
katkalof Jul 27, 2016
3ef74f4
remove excess test
katkalof Aug 9, 2016
3efbfa9
add onlayout visibility==gone check
katkalof Aug 9, 2016
d666b53
fix wrong work with 2 and more element and layout padding, add exampl…
katkalof Aug 9, 2016
bc0416a
fix wrong work with 2 and more element and layout padding, add exampl…
katkalof Aug 10, 2016
4f1158b
check divide by zero exception, when not match_parent children
katkalof Aug 10, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion app/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ android {
buildToolsVersion versions.buildTools

defaultConfig {
applicationId 'ru.yandex.yamblz'
applicationId 'ru.yandex.yamblz_customviewgroup'
minSdkVersion versions.minSdk
targetSdkVersion versions.targetSdk
versionCode versions.code // Notice that you may want to use BUILD_NUMBER from CI in real project with own CI.
Expand Down
7 changes: 7 additions & 0 deletions app/src/main/java/ru/yandex/yamblz/App.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@
import android.content.Context;
import android.support.annotation.NonNull;

import com.facebook.stetho.Stetho;

import ru.yandex.yamblz.developer_settings.DevMetricsProxy;
import ru.yandex.yamblz.developer_settings.DeveloperSettingsModel;
import timber.log.Timber;
Expand Down Expand Up @@ -31,6 +33,11 @@ public void onCreate() {
DevMetricsProxy devMetricsProxy = applicationComponent.devMetricsProxy();
devMetricsProxy.apply();
}
Stetho.initialize(
Stetho.newInitializerBuilder(this)
.enableDumpapp(Stetho.defaultDumperPluginsProvider(this))
.enableWebKitInspector(Stetho.defaultInspectorModulesProvider(this))
.build());
}

@NonNull
Expand Down
97 changes: 97 additions & 0 deletions app/src/main/java/ru/yandex/yamblz/ui/layouts/CustomLayout.java
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
package ru.yandex.yamblz.ui.layouts;

import android.content.Context;
import android.util.AttributeSet;
import android.view.View;
import android.view.ViewGroup;

import java.util.ArrayList;

public class CustomLayout extends ViewGroup {
private ArrayList<View> mMatchParentChildren = new ArrayList<>();

public CustomLayout(Context context) {
this(context, null, 0);
}

public CustomLayout(Context context, AttributeSet attrs) {
this(context, attrs, 0);
}

public CustomLayout(Context context, AttributeSet attrs, int defStyleAttr) {
super(context, attrs, defStyleAttr);
}

@Override
protected void onMeasure(int widthMeasureSpec, int heightMeasureSpec) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Нужно учитывать паддинги и при измерении

Copy link
Author

@katkalof katkalof Aug 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

согласен, мой косяк
просто сравнив с linear'ом получил одинаковый результат(на скриншоте первом видно), и не стал задумываться
видимо тест не очень удачный
при первой возможности пофикшу

Copy link
Author

Choose a reason for hiding this comment

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

потыкал паддинги в методе измерения
ни к чему хорошему это не приводит)
наверно потому, что за меня это решает метод измерения суперкласса

Copy link
Collaborator

Choose a reason for hiding this comment

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

А можешь показать, где это супер класс делает?

Copy link
Author

@katkalof katkalof Aug 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Отказать, первое предположение было неверно
Осознал
Я вызываю для детей measureChild и он там вызывает getChildMeasureSpec и он думает, что я передаю родительские MeasureSpec поэтому хватает падинги родителя и смело из с их учетом расчитывает чилда.
И если это так и происходит для первых детей, то для детей которые match_parent это не так.

Это все будет работать до тех пор, пока match_parent в лейауте один. :)
то есть по сути задание выполнено)))
но надо бы исправить, чтобы для двух

Copy link
Collaborator

@bracadabra bracadabra Aug 10, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Так view.measure не учитывает паддинги, паддинги учитывает measureChild, поэтому просто измерится по ширине и высоте родительской вьюхи без каких либо ограничений, а при отрисовке паддинг учитывается.

Copy link
Author

Choose a reason for hiding this comment

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

да я про прошлый случай и рассказывал, про measureChild. Типа почему это все работало раньше :)
Пока тыкался сейчас, понял, что забыл проверять отсутствие match_parent элементов и делил на 0. Видимо, забыл добавить проверку, когда решил, что у меня может быть их больше одного. so sad
пофиксил

Copy link
Collaborator

Choose a reason for hiding this comment

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

разобрались) в общем, это просто тонкий момент, который нужно в голове держать

Copy link
Author

Choose a reason for hiding this comment

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

понял. спасибо, было познавательно)
как-нибудь на досуге буду разбираться еще с мерджинами потомков, чтобы совсем проникнуться)

Copy link
Collaborator

Choose a reason for hiding this comment

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

без проблем

int totalWidth = 0;
int layoutWidth = MeasureSpec.getSize(widthMeasureSpec);
//Высота лейаута по высоте максимального элемента
int maxHeight = MeasureSpec.getSize(heightMeasureSpec);
int changedHeightMeasureSpec = heightMeasureSpec;

//Родитель может вызвать onMeasure не единожды
mMatchParentChildren.clear();


for (int i = 0; i < getChildCount(); ++i) {
View child = getChildAt(i);

if (child == null) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

А в какой ситуации такое может быть?

Copy link
Author

Choose a reason for hiding this comment

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

Я думаю, например, можно добавить программно вьюшку к лейауту зануленую, или удалить неправильно.
Читал исходники Linear'а, там есть подобная проверка - решил, что она не лишена смысла

Copy link
Collaborator

@bracadabra bracadabra Aug 9, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Если добавлять зануленную вьюху, то есть обычный null, то вылетет IllegaдArgumentException. Я посмотрел мельком LinealLayout, там вызвается getVirtualChildAt, судя по комменту, он используется в TableRow для каких-то выравниваний, то есть не подходит для нашего случая.

Copy link
Author

Choose a reason for hiding this comment

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

там под личиной метода getVirtualChildAt скрывается getChildAt(index)
по-этому я решил, что будет лучше перестраховаться и оставить данную проверку

Copy link
Collaborator

Choose a reason for hiding this comment

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

да, то есть использует реализацию по умолчанию, но вообще метод существует для TableRow, который видимо и может вернуть null.

Copy link
Author

Choose a reason for hiding this comment

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

осознал)
надо было внимательнее читать документацию)

Copy link
Author

Choose a reason for hiding this comment

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

fix

continue;
}
if (child.getVisibility() == GONE) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

неплохо бы проверять скрыта ли вьюха и в onLayout

Copy link
Author

Choose a reason for hiding this comment

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

fix

continue;
}

switch (child.getLayoutParams().width) {
case LayoutParams.MATCH_PARENT:
mMatchParentChildren.add(child);
break;
case LayoutParams.WRAP_CONTENT:
default:
measureChild(child, widthMeasureSpec, heightMeasureSpec);
totalWidth += child.getMeasuredWidth();
int measuredHeight = child.getMeasuredHeight();
if (maxHeight < measuredHeight) {
maxHeight = measuredHeight;
changedHeightMeasureSpec = MeasureSpec.makeMeasureSpec(maxHeight, MeasureSpec.UNSPECIFIED);
Copy link
Collaborator

Choose a reason for hiding this comment

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

UNSPECIFIED по идее не подразумевает никаких ограничений

Copy link
Author

Choose a reason for hiding this comment

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

Ну да, у меня и нет желания его ограничивать по высоте.
Пусть будет такая, сколько попросит самый высокий

Copy link
Collaborator

Choose a reason for hiding this comment

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

Это я к тому, что раз ограничений нет и maxHeight ни на что не влияет, то можно его и не передавать

Copy link
Author

Choose a reason for hiding this comment

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

смысл в том, что по maxHeight будет определена высота самого лейаута потом

Copy link
Collaborator

Choose a reason for hiding this comment

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

да, верно, значит мой коммент неактуален

}
}
}

//Если осталось место или оно не ограничено, почему бы не показать matchParent
if ((totalWidth < layoutWidth) || widthMeasureSpec == MeasureSpec.makeMeasureSpec(0, MeasureSpec.UNSPECIFIED)) {
//В случае, если не знаем ширина родителя не остановленна,используем вдвое увеличенную ширину не match_parent элементов
layoutWidth = layoutWidth != 0 ? layoutWidth : totalWidth * 2;
Copy link
Collaborator

Choose a reason for hiding this comment

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

почему не утроенную ширину?

Copy link
Author

Choose a reason for hiding this comment

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

Можно и утроенную)
Тут я размышлял о случае, когда вьюгрупп лежит в скроле
Получается надо как-то ограничивать match_parent чилда, либо вообще не давать, либо давать сколько-то

Copy link
Collaborator

Choose a reason for hiding this comment

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

Да, три тоже хорошее число) мне кажется, если в сумме все потомки превышают ширину экрана, то ширина match_parent вьюхи, должна быть просто 0, так как места для нее нет. Так получается более предсказуемо, чем умножать на 2, 3 и любое другое число.

Copy link
Author

Choose a reason for hiding this comment

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

=)
я делаю её для описанного тобой случая)
а потом решил придумать что-то поинтересней для скролвью, жалко стало вьюху)
согласен, что не супер вариант, но это же кастом :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

ну да, теперь зачитаться можно)

Copy link
Author

Choose a reason for hiding this comment

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

сорян) у меня фантазия разыгралась тогда)

//Остаток разделям между оставшимися детьми
int mpChildWidth = (layoutWidth - totalWidth) / mMatchParentChildren.size();
int childWidthMeasureSpec = MeasureSpec.makeMeasureSpec(mpChildWidth, MeasureSpec.EXACTLY);
for (int i = 0; i < mMatchParentChildren.size(); i++) {
measureChild(mMatchParentChildren.get(i), childWidthMeasureSpec, changedHeightMeasureSpec);
totalWidth += mMatchParentChildren.get(i).getMeasuredWidth();
int measuredHeight = mMatchParentChildren.get(i).getMeasuredHeight();
if (maxHeight < measuredHeight) {
maxHeight = measuredHeight;
changedHeightMeasureSpec = MeasureSpec.makeMeasureSpec(maxHeight, MeasureSpec.AT_MOST);
}
}
}
super.onMeasure(MeasureSpec.makeMeasureSpec(layoutWidth, MeasureSpec.EXACTLY), changedHeightMeasureSpec);
}

@Override
protected void onLayout(boolean changed, int l, int t, int r, int b) {

int horizontalOffset = getPaddingLeft();
int verticalOffset = getPaddingTop();
int childCount = getChildCount();
for (int i = 0; i < childCount; ++i) {
View child = getChildAt(i);
int childWidth = child.getMeasuredWidth();
int childHeight = child.getMeasuredHeight();
child.layout(horizontalOffset, verticalOffset, horizontalOffset + childWidth, verticalOffset + childHeight);
horizontalOffset += childWidth;
}
}
}
29 changes: 19 additions & 10 deletions app/src/main/res/layout/fragment_content.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,14 +1,23 @@
<?xml version="1.0" encoding="utf-8"?>
<FrameLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent">
<ScrollView xmlns:android="http://schemas.android.com/apk/res/android"

<TextView
android:layout_width="match_parent"
android:layout_height="match_parent">
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:text="@string/content"
android:textSize="42sp"
android:gravity="center"
/>
android:layout_height="wrap_content"
android:orientation="vertical">

</FrameLayout>

<include layout="@layout/inner_test_1"/>
<include layout="@layout/inner_test_2"/>
<include layout="@layout/inner_test_3"/>
<include layout="@layout/inner_test_5"/>
<include layout="@layout/inner_test_6"/>
<include layout="@layout/inner_test_7"/>
<include layout="@layout/inner_test_8"/>
<include layout="@layout/inner_test_9"/>
<include layout="@layout/inner_test_10"/>

</LinearLayout>
</ScrollView>
85 changes: 85 additions & 0 deletions app/src/main/res/layout/inner_test_1.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical">

<TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="wrap_content LL and CL with expecly width for all" />

<LinearLayout
android:layout_width="wrap_content"
android:layout_height="100dp">

<TextView
android:layout_width="10dp"
android:layout_height="match_parent"
android:background="@color/gray" />

<TextView
android:layout_width="100dp"
android:layout_height="match_parent"
android:background="@color/colorAccent" />

<TextView
android:layout_width="50dp"
android:layout_height="match_parent"
android:background="@color/colorPrimary" />

<TextView
android:layout_width="70dp"
android:layout_height="match_parent"
android:background="@color/colorPrimaryDark" />

<TextView
android:layout_width="1000dp"
android:layout_height="match_parent"
android:background="@color/d2m_font_default_description" />

</LinearLayout>


<TextView
android:layout_width="match_parent"
android:layout_height="1dp"
android:background="@color/black" />

<ru.yandex.yamblz.ui.layouts.CustomLayout
android:layout_width="wrap_content"
android:layout_height="100dp">

<TextView
android:layout_width="10dp"
android:layout_height="match_parent"
android:background="@color/gray" />

<TextView
android:layout_width="100dp"
android:layout_height="match_parent"
android:background="@color/colorAccent" />

<TextView
android:layout_width="50dp"
android:layout_height="match_parent"
android:background="@color/colorPrimary" />

<TextView
android:layout_width="70dp"
android:layout_height="match_parent"
android:background="@color/colorPrimaryDark" />


<TextView
android:layout_width="1000dp"
android:layout_height="match_parent"
android:background="@color/d2m_font_default_description" />

</ru.yandex.yamblz.ui.layouts.CustomLayout>

<TextView
android:layout_width="match_parent"
android:layout_height="3dp"
android:background="@color/black" />
</LinearLayout>
90 changes: 90 additions & 0 deletions app/src/main/res/layout/inner_test_10.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
<?xml version="1.0" encoding="utf-8"?>
<LinearLayout xmlns:android="http://schemas.android.com/apk/res/android"
android:layout_width="match_parent"
android:layout_height="match_parent"
android:orientation="vertical">

<TextView
android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:text="LL and CL with child margin"
/>

<LinearLayout
android:layout_width="match_parent"
android:layout_height="100dp"
>

<TextView
android:layout_width="10dp"
android:layout_height="match_parent"
android:background="@color/gray" />

<TextView
android:layout_width="100dp"
android:layout_height="match_parent"
android:background="@color/colorAccent" />

<TextView
android:layout_width="0dp"
android:layout_weight="1"
android:layout_margin="8dp"
android:layout_height="match_parent"
android:background="@color/colorPrimary" />

<TextView
android:layout_width="70dp"
android:layout_height="match_parent"
android:background="@color/colorPrimaryDark" />

<TextView
android:layout_width="50dp"
android:layout_height="match_parent"
android:background="@color/d2m_font_default_description" />

</LinearLayout>


<TextView
android:layout_width="match_parent"
android:layout_height="1dp"
android:background="@color/black" />

<ru.yandex.yamblz.ui.layouts.CustomLayout
android:layout_width="wrap_content"
android:layout_height="100dp">

<TextView
android:layout_width="10dp"
android:layout_height="match_parent"
android:background="@color/gray" />

<TextView
android:layout_width="100dp"
android:layout_height="match_parent"
android:background="@color/colorAccent" />

<TextView
android:layout_width="match_parent"
android:layout_height="match_parent"
android:layout_margin="8dp"
android:background="@color/colorPrimary" />

<TextView
android:layout_width="70dp"
android:layout_height="match_parent"
android:background="@color/colorPrimaryDark" />


<TextView
android:layout_width="50dp"
android:layout_height="match_parent"
android:background="@color/d2m_font_default_description" />

</ru.yandex.yamblz.ui.layouts.CustomLayout>

<TextView
android:layout_width="match_parent"
android:layout_height="3dp"
android:background="@color/black" />
</LinearLayout>
Loading