roubachof / Sharpnado.CollectionView

A performant list view supporting: grid, horizontal and vertical layout, drag and drop, and reveal animations.
The Unlicense
244 stars 30 forks source link

Carousel offset is not calculated correctly when height is greater than width #83

Closed ems107 closed 1 year ago

ems107 commented 1 year ago

I think this is related to this issue #76

From what I've seen debugging, in the ScrollToCurrentItem() method of Android Renderer, Control.MeasuredWidth will always be 0 when the height is greater than width (and offset will always be negative).

private void ScrollToCurrentItem()
{
    ...
    int offset = 0;
    if (HorizontalLinearLayoutManager != null)
    {
        int itemWidth = PlatformHelper.Instance.DpToPixels(
            Element.ItemWidth
            + Element.ItemSpacing
            + Element.CollectionPadding.HorizontalThickness);

        int width = Control.MeasuredWidth;

        switch (Element.SnapStyle)
        {
            case SnapStyle.Center:
                offset = (width / 2) - (itemWidth / 2);
                break;
        }
    }

    LinearLayoutManager?.ScrollToPositionWithOffset(Element.CurrentIndex, offset);
    GridLayoutManager?.ScrollToPositionWithOffset(Element.CurrentIndex, offset);
}

But when the width is greater than height, OnPreDraw() will call ScrollToCurrentItem() one more time, this time with values in the width and height that are used.

private void OnPreDraw(object sender, ViewTreeObserver.PreDrawEventArgs e)
{
    ...
    if (Control.Height < Control.Width)
    {
        if (!_isLandscape)
        {
            orientationChanged = true;
            _isLandscape = true;

            // Has just rotated
            if (HorizontalLinearLayoutManager != null)
            {
                ScrollToCurrentItem();
            }
        }
    }
    ...
}

So the problem here is that ScrollToCurrentItem() never is called with height and width values when the height is greater than width.

With the changes, inside the OnLayout() override, base.OnLayout() is called before ComputeItemSize() so the Control will have height and width values when ScrollToCurrentItem() is evaluated.

Before changes: image

After changes: image

roubachof commented 1 year ago

Man thank you for all those awesome PRs. I'm currently at the middle of the migration of the component to maui. I have created the new sample app and it's working fine on Android. Nevertheless, I don't when I will be able to merge all those and finish the migration since I have a lot of work right now. My guess is probably a month or so.

ems107 commented 1 year ago

Don't worry, review it and merge it when you can. Thank you for sharing this repo, I wish I had found it earlier.

roubachof commented 1 year ago

Hi I am currently debugging the ios MAUI version of the collection view, and so was going through your PRS. I noticed that only the android implementation is done. Unfortunately I cannot merge an incomplete implementation.

roubachof commented 1 year ago

integrated in next version