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

More modular AllControls sample #677

Merged
merged 16 commits into from
Mar 9, 2020

Conversation

TimLariviere
Copy link
Member

@TimLariviere TimLariviere commented Feb 23, 2020

I'm reworking the AllControls sample to make it easier to read a specific sample (e.g. How to use SkiaSharp?) and as well as make it easier to add new samples to it.

The idea is to add have distinct files and register them in the Samples.fs file, so the app will automatically pick them up.

Currently moved SkiaSharp, OxyPlot and Maps

@SergejDK Let me know what you think of it

Screen-Recording-2020-02-23-at-7

@SergejDK
Copy link
Collaborator

@TimLariviere
I think this is a great idea. The current AllControls-sample was getting way to big to handle and no one could find a specific implementation easily. This even shows how to build bigger applications with more screens :)

I'll take a look on what exactly you did in the next days.

@TimLariviere
Copy link
Member Author

Finished moving samples away from the big file. :)
Tested it on iOS for now. Other platforms might not build because of dependencies to extensions.

@TimLariviere TimLariviere changed the title [WIP] More modular AllControls sample More modular AllControls sample Feb 28, 2020
@SergejDK
Copy link
Collaborator

SergejDK commented Mar 2, 2020

@TimLariviere
Works amazing and pretty good examples especially for the extensions. Code looks good too even though I needed a bit to understand it :)

Tested on:

  • iOS -> works good
  • android -> currently my setup is kinda broke need to check this before i can test it...
  • macOs ->
    • ScrollView Scroll-event not working
    • Swipeview / Refreshview not working but I think mac is not supported
    • Can't get back from navigationpage
  • GTK / WPF / UWP -> Could not test because of my current setup

Maybe one more thing - we should show the control in die videomanagerextensions in this example else the sound keeps on playing which is kinda annoying.

@TimLariviere
Copy link
Member Author

@SergejDK Thanks for the feedback!

Code looks good too even though I needed a bit to understand it :)

Do you have any suggestion how we can make this part easier to understand?

macOS: Can't get back from navigationpage

Yes, I noticed this as well. I wasn't expecting Android/iOS to allow having a Shell/NavigationPage inside another NavigationPage but it worked. Though macOS doesn't allow this.
Will need to add the famous "Go back" button manually for macOS only.

Maybe one more thing - we should show the control in die videomanagerextensions in this example else the sound keeps on playing which is kinda annoying.

Noticed that as well.
For some reasons, the video player doesn't get disposed when navigating back.
Need to investigate this.

@SergejDK
Copy link
Collaborator

SergejDK commented Mar 4, 2020

Do you have any suggestion how we can make this part easier to understand?

I just think that a little code documentation is enough - just did not worked a lot with f# the last weeks.

Yes, I noticed this as well. I wasn't expecting Android/iOS to allow having a Shell/NavigationPage inside another NavigationPage but it worked. Though macOS doesn't allow this.
Will need to add the famous "Go back" button manually for macOS only.

This should not be to difficult - maybe seperate issue good first issue ?

@TimLariviere TimLariviere merged commit 82dd091 into fabulous-dev:master Mar 9, 2020
@TimLariviere TimLariviere deleted the clean-allcontrols branch March 9, 2020 12:47
@TimLariviere TimLariviere mentioned this pull request Apr 17, 2020
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.

2 participants