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.

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

[Android] Fix CollectionView EmptyView #11763

Merged
merged 17 commits into from
May 8, 2023
Merged

[Android] Fix CollectionView EmptyView #11763

merged 17 commits into from
May 8, 2023

Conversation

jsuarezruiz
Copy link
Contributor

Description of Change

Fix Android CollectionView EmptyView.

Captura de pantalla 2022-11-30 a las 14 51 19

Issues Fixed

Fixes #10819

@jsuarezruiz jsuarezruiz added t/bug Something isn't working platform/android 🤖 area-controls-collectionview CollectionView, CarouselView, IndicatorView labels Nov 30, 2022
Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

Questions about the empty view size changes.

if (!double.IsInfinity(height))
height = Context.ToPixels(height);

UpdateEmptyViewSize(width, height);
Copy link
Contributor

Choose a reason for hiding this comment

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

This will set the empty view size to (widthConstraint, heightConstraint) - that's exactly what Jon's comment was worried about. Shouldn't it use the result of the measurement?

Also, is it even necessary to update the empty view size during measure? It's already being updated during Arrange.

Copy link
Member

Choose a reason for hiding this comment

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

@jsuarezruiz the conversation was mark as resolved, but does not look like it was, can you please comment?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You are right, with the current approach the height is infinity. However, with the return values from base.GetDesiredSize:

DesiredSize {Width=0 Height=4793480}

The EmptyView will not be visible using that values (zero Width).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

cc @hartez

@github-actions
Copy link
Contributor

⚠️ Your code has been reformatted. ⚠️

if (!double.IsInfinity(height))
height = Context.ToPixels(height);

UpdateEmptyViewSize(width, height);
Copy link
Contributor

Choose a reason for hiding this comment

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

If the ItemsView measurement is unconstrained in one direction, won't that set the height or width of the EmptyViewAdapter's RecyclerViewWidth or RecyclerViewHeight to infinity? It seems like it should use the height returned from GetDesiredSize, not the measurement constraints.

Copy link
Contributor

@hartez hartez left a comment

Choose a reason for hiding this comment

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

Okay, looks like it's all working.

@hartez hartez merged commit 1d9c5cb into main May 8, 2023
29 checks passed
@hartez hartez deleted the fix-10819 branch May 8, 2023 20:56
@janseris
Copy link

When is this fix released?

@hartez
Copy link
Contributor

hartez commented May 10, 2023

When is this fix released?

At the moment, it's slated to be released as part of .NET 8. So it will likely be in the next .NET 8 preview.

rmarinho pushed a commit that referenced this pull request May 30, 2023
* Fix Android CollectionView EmptyView

* Update src/Controls/src/Core/Handlers/Items/ItemsViewHandler.Android.cs

Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com>

* Fixed build errors

* Auto-format source code

---------

Co-authored-by: E.Z. Hart <hartez@users.noreply.github.com>
Co-authored-by: GitHub Actions Autoformatter <autoformat@example.com>
@FreakyAli
Copy link

Wow, I am surprised that it took 2 years to fix something that is one of the major functionalities of CollectionView, I remember facing this issue with .Net 6 and it's coming out with .Net 8, Damn...

@mattleibow
Copy link
Member

EmptyViewTemplate still does not render at all: #18551

@github-actions github-actions bot locked and limited conversation to collaborators Dec 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-controls-collectionview CollectionView, CarouselView, IndicatorView platform/android 🤖 t/bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android - Collectionview EmptyView not visible
7 participants