Skip to content

Migrating ListView to CollectionView#934

Closed
cscharf wants to merge 8 commits intomasterfrom
collectionview
Closed

Migrating ListView to CollectionView#934
cscharf wants to merge 8 commits intomasterfrom
collectionview

Conversation

@cscharf
Copy link

@cscharf cscharf commented May 29, 2020

Builds but crashes when displaying the ciphers page.

  • Refactored all ListView to CollectionView, including AutofillCiphers, Ciphers and Groupings pages that used that type, as well as Folders, Settings and Password History
  • Refactored CipherViewCell to no longer be a ViewCell but instead a Grid (based on feedback); removed "CellView" from everywhere
  • Removed Android renderers for the CipherViewCell and Extended list views
  • Updated style definitions to point to CollectionView
  • Added separator style as BoxView on necessary collections since CollectionView doesn't support the ListView concept of Separator
  • Added Icon Renderer to utilities instead of coding that directly into cipher view control

Open Issues

  • iOS rendering of Grouped list items is wonky at best, sometimes accordion style collapse of white-space and sometimes not, sometimes different throughout the list, etc.
  • Android - Context menu doesn't work, null reference exception
  • iOS + Android - Padding of items in CollectionView when viewing vault is non-existent and squishes them all together (Grouped view appears slightly differently when there are groups)

@cscharf
Copy link
Author

cscharf commented May 29, 2020

Issue with consistent rendering will need some debugging, but otherwise works on iOS:
image

@cscharf
Copy link
Author

cscharf commented Jun 1, 2020

Android won't even render/display the cipher items in the list, although the CollectionView itself on the Groupings page looks great (unlike iOS)
image

<ItemGroup>
<ProjectReference Include="..\App\App.csproj">
<Project>{9F1742A7-7D03-4BB3-8FCD-41BC3002B00A}</Project>
<Project>{EE44C6A1-2A85-45FE-8D9B-BF1D5F88809C}</Project>
Copy link
Contributor

Choose a reason for hiding this comment

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

VSwin and VSmac seem to argue endlessly about this value. We've established VSwin as our gold standard so this is just a reminder to revert this before finalizing the PR. (fwiw Rider is polite enough to leave it alone)

Copy link
Author

@cscharf cscharf Jun 1, 2020

Choose a reason for hiding this comment

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

Ah, that is odd considering the .sln file actually has the App project listed with UUID, EE44C6A1-2A85-45FE-8D9B-BF1D5F88809C, which VSmac seems to be picking up and correcting in the project reference in the Android project. Not sure where VSwin is getting the UUID, 9F1742A7-7D03-4BB3-8FCD-41BC3002B00A since it exists nowhere in the .sln file or the App.csproj file. very bizarre; but I'll undo it before PR-time.

@cscharf
Copy link
Author

cscharf commented Jun 4, 2020

Latest updates, iOS display issues and inconsistency in padding still broken/not working. Android looks better, however cipher list does not respect padding and context/more menu is broken (null ref exception).

@cscharf
Copy link
Author

cscharf commented Jun 4, 2020

System.NullReferenceException: Object reference not set to an instance of an object.
  at Bit.App.Utilities.AppHelpers.CipherListOptions (Xamarin.Forms.ContentPage page, Bit.Core.Models.View.CipherView cipher) [0x000c6] in /Users/chadscharf/source/bitwarden/vs/mobile/src/App/Utilities/AppHelpers.cs:23
  at Bit.App.Pages.GroupingsPageViewModel.CipherOptionsAsync (Bit.Core.Models.View.CipherView cipher) [0x0004b] in /Users/chadscharf/source/bitwarden/vs/mobile/src/App/Pages/Vault/GroupingsPage/GroupingsPageViewModel.cs:496
  at System.Runtime.CompilerServices.AsyncMethodBuilderCore+<>c.<ThrowAsync>b__7_0 (System.Object state) [0x00000] in /Users/builder/jenkins/workspace/archive-mono/2020-02/android/release/mcs/class/referencesource/mscorlib/system/runtime/compilerservices/AsyncMethodBuilder.cs:1021
  at Android.App.SyncContext+<>c__DisplayClass2_0.<Post>b__0 () [0x00000] in <eaa205f580954a64824b74a79fa87c62>:0
  at Java.Lang.Thread+RunnableImplementor.Run () [0x00008] in <eaa205f580954a64824b74a79fa87c62>:0
  at Java.Lang.IRunnableInvoker.n_Run (System.IntPtr jnienv, System.IntPtr native__this) [0x00008] in <eaa205f580954a64824b74a79fa87c62>:0
  at at (wrapper dynamic-method) Android.Runtime.DynamicMethodNameCounter.1(intptr,intptr)

@CLAassistant
Copy link

CLAassistant commented Jul 21, 2020

CLA assistant check
All committers have signed the CLA.

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.

3 participants