NobsterTheLobster / Xamarin.Forms.GridView

GridView for xamarin forms
MIT License
60 stars 10 forks source link

Issue with header height #8

Closed MatthieuNarcisi closed 7 years ago

MatthieuNarcisi commented 7 years ago

I have an issue with the header size of the GridView, as I read in the code, the header should be calculated from the first header and then applied to all the other headers.

But my experience was that the default size (let's say 50) was applied no matter what I tried in my code. I have done the following :

But none of it worked. So I added a new property on the GridView control : HeaderHeight which enables me to define the heights of the headers of each group. I then changed the code of the renderer to use it instead of the one given by the XF Renderer. Like so :

On Android (GridViewAdapter, on the OnCreateViewHolder method) :

var height = viewType == 0 ? Convert.ToInt32(this.Element.RowHeight) : Convert.ToInt32(this.Element.HeaderHeight); // Convert.ToInt32(gridViewCell.RenderHeight);
var width = Convert.ToInt32(this.Element.MinItemWidth);
var dpW = this.ConvertDpToPixels(width);
var dpH = this.ConvertDpToPixels(height);

On iOS (GridViewRenderer, on the ReloadData method) :

// Set the reference size accordingly.
flowLayout.HeaderReferenceSize = new CGSize(Element.Width, Element.HeaderHeight);

and OnElementChanged:

// Set the reference size accordingly.
flowLayout.HeaderReferenceSize = new CGSize(Element.Width, Element.HeaderHeight);

Am I the only one to have this issue, and in that case, am I doing something wrong ?

NobsterTheLobster commented 7 years ago

"as I read in the code, the header should be calculated from the first header and then applied to all the other headers" Yep on ios that's certainly what its meant to do and on android I believed it to be dynamic. Last time I checked it appeared to be behaving correctly on both platforms but I don't have the ability to recheck it right now so maybe there is an issue. When you say you tried setting the height on the first child you are talking within the groupheadertemplate presumably.

   <controls:GridView.GroupHeaderTemplate>
            <DataTemplate>
                <controls:GridViewXamlCell>
                    <Grid Padding="5" HeightRequest="100">
                        <Label FontSize="Medium" Text="{Binding Key,StringFormat='GroupID:{0}'}" TextColor="Accent"/>
                    </Grid>
                </controls:GridViewXamlCell>
            </DataTemplate>
        </controls:GridView.GroupHeaderTemplate>

EDIT: "Set the height or the rows and set the HasUnevenRows to true" that would be what you'd need to do for UWP since the default renderer is being used but HasUnvenRows has no effect in the ios or android renders.

MatthieuNarcisi commented 7 years ago

Yes, I did exactly what you put on your example. And it did not seem to change the height of the header, so I had to resort on adding this property.

As I understand, I will remove the HasUnevenRows from my code.

NobsterTheLobster commented 7 years ago

I've just updated the readme because I realise that the HasUnevenRows usage wasn't clear.

NobsterTheLobster commented 7 years ago

I'll hopefully get a chance to retest the project tomorrow. As a matter of interest what version of xamarin forms are you using. I seem to recall some issues in relation to cell height with the xamarin's own listview in certain versions.

MatthieuNarcisi commented 7 years ago

Yeah you are right, it might be possible, I have been using several versions of Xamarin.Forms (I try to keep up to date). But the main ones were 2.3.4.* and the last one : 2.3.4.247.

I tried to look for a bug in Xamarin Forms also but could not find people complaining online.

NobsterTheLobster commented 7 years ago

Sorry for the delay I've confirmed your issue on android at least. RenderHeight always returns 40 for some reason. I am in the process of updating my development environment but hopefully I will have an update this week.

NobsterTheLobster commented 7 years ago

I've made a change to the repo but its not the same as you have above so you might want to stay clear of it. Instead of adding a new property, I've instead chosen to explicitly take the HeightRequest property as declared in the view. Your solution might be better but leveraging the height request from the XAML means its also compatible with the UWP renderer. If HeightRequest is not defined in the view then it fails over to render height as to maintain backwards compatibility

ANDROID

 var height = viewType == 0 ? Convert.ToInt32(Element.RowHeight) :
                gridViewCell.View.HeightRequest != -1 ?
                Convert.ToInt32(gridViewCell.View.HeightRequest) :
                Convert.ToInt32(gridViewCell.RenderHeight);

IOS

flowLayout.HeaderReferenceSize = new CGSize(Element.Width, cell.View.HeightRequest != -1 ?cell.View.HeightRequest : cell.RenderHeight);

So you can now do the following and it will be respected.

 <controls:GridView.GroupHeaderTemplate>
            <DataTemplate>
                <controls:GridViewXamlCell>
                    <Grid HeightRequest="50">
                        <Label FontSize="Large"  VerticalTextAlignment="Center" Text="{Binding Key,StringFormat='GroupID:{0}'}" TextColor="Accent"/>
                    </Grid>
                </controls:GridViewXamlCell>
            </DataTemplate>
        </controls:GridView.GroupHeaderTemplate>
MatthieuNarcisi commented 7 years ago

Great news, I will update my project. It is different from what I did, but I think it is really ok, I see no issue with the way you did it. Thanks a lot.