jfversluis / been-pwned

App that leverages the haveibeenpwned.com API by Troy Hunt. This app is available in the App Stores and is used in several of my talks as well as my book Xamarin.Forms Essentials.
MIT License
16 stars 3 forks source link

Description field of breach no dynamic height #32

Closed jfversluis closed 7 years ago

jfversluis commented 7 years ago

The field which holds the description of a breach is not dynamic in height and can therefore be too small. One example of this is Experian.

screenshot 2017-08-17 11 31 56

The content is cut off at the bottom, which does not look pretty.

sthewissen commented 7 years ago

There is a piece of code for this in place already except it doesn't work as it should.

https://github.com/jfversluis/been-pwned/blob/master/src/BeenPwnedApp/iOS/Renderers/ExtendedWebViewRenderer.cs#L51

jfversluis commented 7 years ago

Or does it and doesn't the cell get resized accordingly?

sthewissen commented 7 years ago

Might also be the case, I just kind of gave up at some point.

Depechie commented 7 years ago

Just did some tests... Placing the WebView inside the TableView will no longer help resizing the WebView if needed. Now it takes the HeightRequest set in the Xaml > 200 ( https://github.com/jfversluis/been-pwned/blob/master/src/BeenPwnedApp/Core/Pages/BreachPage.xaml#L36 )

If the WebView is outside of the TableView, you can alter the size according to the content loaded. But there is a problem with content underneath TableViews, because tableviews add extra padding if there is space on the page... Cfr https://stackoverflow.com/questions/44965804/tableview-height-wrapping-xamarin-forms and https://forums.xamarin.com/discussion/33629/how-to-make-tableview-fit-to-content-with-no-extra-rows

So general suggestion, redesign page to be full ListView ( as Jason Smith also suggest for no longer using TableView if you want performance ;) ).

sthewissen commented 7 years ago

I do wonder how this screen would be designed and look good using a ListView. Can't quite paint a picture of that in my head yet :P

jfversluis commented 7 years ago

Probably non-scrolling listviews simulating a TableView like in the Evolve app.

But we could of course choose to lose this design altogether ;)

sthewissen commented 7 years ago

True, but whatever we come up with I think it should be about as functional as the current design. The current one gives a nice overview of all the information you want to know.

Depechie commented 7 years ago

Maybe someone else can try again to crack the resizing bug? Anyhow, normally you should be able to mimic the tableview design with a listview. But will take a bit of work of course :/

sthewissen commented 7 years ago

I took a shot at a minor rework for this screen, what do you guys think?

https://user-images.githubusercontent.com/2419439/29454315-f275fa14-840c-11e7-9789-612459715544.png https://user-images.githubusercontent.com/2419439/29454316-f28e15f4-840c-11e7-95b3-98d87794e134.png

jfversluis commented 7 years ago

Ho-ly shhhh... Looks good dude!

Depechie commented 7 years ago

Great!!

jfversluis commented 7 years ago

Implemented and merged with #39, case closed!

sthewissen commented 7 years ago

Thankyou! :kissing_closed_eyes: