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

Workaround for off-screen draw bug in XF4.5+ #1447

Merged
merged 2 commits into from
Jun 30, 2021
Merged

Conversation

mpbw2
Copy link
Contributor

@mpbw2 mpbw2 commented Jun 30, 2021

An issue where a BindableLayout that is modified off-screen isn't re-drawn to reflect the modification (Xamarin Forms 4.5+) broke the way we render organization & collection selection when adding new vault items. This PR works around that in an interesting way.

  • HasCollections is set to true in order to make the layout visible before modifying the content
  • After Collections is updated with the new content, it is set to a new instance of ExtendedObservableCollection containing the values already established in ResetWithRange (which must still occur or this has no effect)
  • Upon scrolling the screen, the collections RepeaterView.ItemsSource is repeatedly set to Collections (limited by a timer to prevent scroll events from max'ing out the thread pool)

Things to consider:

  • All of these seemingly useless steps are required to force collections to draw like they did before we updated to Forms 5.x. A lot of whittling took place to narrow this down.
  • Android exhibits this bug far more consistently than iOS, but I didn't restrict the workaround since I witnessed it happen a few times in iOS.
  • When the owner is changed to an organization and you begin to scroll, you may experience an initial lag in the scroll action as the workaround does its "magic".

Note to my future self: Please for the love of ham, revert this when that bug is fixed

@mpbw2 mpbw2 linked an issue Jun 30, 2021 that may be closed by this pull request
@mpbw2 mpbw2 requested review from cscharf and a team June 30, 2021 19:19
@cscharf
Copy link

cscharf commented Jun 30, 2021

Note to my future self: Please for the love of ham, revert this when that bug is fixed

@mportune-bw , please create a tracking item in Asana for this assigned to yourself with a due date for say, 15 days, hopefully someone on the Xamarin team fixes this soon, but perhaps a reminder on the calendar every 15 days to follow up and a task in Asana to keep track of it would be worthwhile.

Copy link

@cscharf cscharf left a comment

Choose a reason for hiding this comment

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

I think this is okay, just one quick question

@@ -798,15 +802,32 @@ private void OrganizationChanged()
}
if (Cipher.OrganizationId != null)
{
HasCollections = true;
Copy link

Choose a reason for hiding this comment

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

So this is forced true instead of using cols.Any() due to the drawing issue?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, though you're correct I should be using cols.Any() in case the org has no collections. I just need to do it here (before cols is applied to the RepeaterView to make sure the view is visible before modifying data). Good catch, stand by for update...

@mpbw2 mpbw2 requested a review from cscharf June 30, 2021 20:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Android Beta - Unable to save an Item to an Org
2 participants