Discussion: Should one use the 3rd party CarouselView or Xamarin Forms' implementation#33
Open
kapseliboi wants to merge 3 commits intochaosifier:masterfrom
Open
Conversation
Also removed CarouselView dependency
kapseliboi
commented
Dec 10, 2019
| <ErrorReport>prompt</ErrorReport> | ||
| <WarningLevel>4</WarningLevel> | ||
| <AndroidLinkMode>None</AndroidLinkMode> | ||
| <AndroidSupportedAbis>arm64-v8a;armeabi-v7a;x86;x86_64</AndroidSupportedAbis> |
Author
There was a problem hiding this comment.
I was unable to build the sample app without this line.
kapseliboi
commented
Dec 10, 2019
| NewPosition = SelectedTabIndex, | ||
| OldPosition = oldPosition | ||
| }; | ||
| var positionChangedArgs = _positionChangedEventArgsInfo.Invoke( |
Author
There was a problem hiding this comment.
PositionChangedEventArgs has an internal constructor so this is a "hacky" way to call it.
There was a problem hiding this comment.
@kapseliboi you do not have to remove the PositionChangedEventArgs definition in the Xam.Plugin.TabView namespace nor to use the internal constructor:
Just use the fully qualified name "Xamarin.Forms.PositionChangedEventArgs" in the _carouselView_PositionSelected method parameter and continue using Xam.Plugin.TabView.PositionChangedEventArgs for the rest (fully qualified name is not required for this one i think)
That would prevent the API breakage
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
I was unable to use the CarouselView by alexrainman successfully on Linux. I always ended up getting runtime crashes on application startup when running this NuGet package. That's why I had to convert this package to use Xamarin Forms' implementation of CarouselView. I guess it has a lot less functionality and this PR is definitely going to cause some regression so this shouldn't even be accepted as is.
For example I tested with the sample application that pressing next and previous had some weird behavior with the tab that's 2nd by default on the sample app. It was skipped when either pressing next or previous depending on something.
But if this package ever decides to use Xamarin Forms' CarouselView then this code could be helpful.
There is a lot of changes to package configuration files by Rider which I can't really say whether or not it's good, bad or unnecessary.