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

React Native Windows Support #45

Merged
merged 9 commits into from
Mar 11, 2017
Merged

Conversation

ryanlntn
Copy link
Contributor

@ryanlntn ryanlntn commented Mar 8, 2017

Adds support for react-native-windows UWP and WPF.
Depends on upstream PR to RNW: microsoft/react-native-windows#1050

README.md Outdated
@@ -95,7 +95,10 @@ react-native link react-native-view-shot

#### Windows

No support yet. Feel free to PR.
1. In Visual Studio, in the solution explorer, right click on your project solution then select `Add` ➜ `ExisitingProject`

Choose a reason for hiding this comment

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

s/project solution/solution

README.md Outdated
No support yet. Feel free to PR.
1. In Visual Studio, in the solution explorer, right click on your project solution then select `Add` ➜ `ExisitingProject`
2. Go to `node_modules` ➜ `react-native-view-shot` and add `RNViewShot.csproj` (UWP) or optionally `RNViewShot.Net46.csproj` (WPF)
3. In Visual Studio, in the solution explorer, right click on your project then select `Add` ➜ `Reference`

Choose a reason for hiding this comment

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

might want to say "Application project" here to be clear

<TargetFrameworkVersion>v4.6</TargetFrameworkVersion>
<FileAlignment>512</FileAlignment>
</PropertyGroup>
<PropertyGroup Condition=" '$(Configuration)|$(Platform)' == 'Debug|AnyCPU' ">

Choose a reason for hiding this comment

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

need to get rid of the dreaded AnyCPU. x86, x64, and ARM only for RNW.

}
catch (Exception ex)
{
Console.WriteLine(ex.ToString());

Choose a reason for hiding this comment

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

use Debug.WriteLine() from the System.Diagnostics namespace instead.


private async Task<StorageFile> GetStorageFile()
{
StorageFolder storageFolder = ApplicationData.Current.LocalFolder;

Choose a reason for hiding this comment

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

var

using (InMemoryRandomAccessStream ras = new InMemoryRandomAccessStream())
{
await CaptureView(view, ras);
byte[] imageBytes = new byte[ras.Size];

Choose a reason for hiding this comment

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

var

using (InMemoryRandomAccessStream ras = new InMemoryRandomAccessStream())
{
await CaptureView(view, ras);
byte[] imageBytes = new byte[ras.Size];

Choose a reason for hiding this comment

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

var

}

UIManagerModule uiManager = this._reactContext.GetNativeModule<UIManagerModule>();
uiManager.AddUIBlock(new ViewShot(tag, format, quality, width, height, path, result, promise));

Choose a reason for hiding this comment

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

extract new ViewShot to a variable for easier debugger inspection

using (InMemoryRandomAccessStream ras = new InMemoryRandomAccessStream())
{
await CaptureView(view, ras);
StorageFile file = await GetStorageFile();

Choose a reason for hiding this comment

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

var

}
else if ("data-uri" == result)
{
using (InMemoryRandomAccessStream ras = new InMemoryRandomAccessStream())

Choose a reason for hiding this comment

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

var

{
string fileName = Guid.NewGuid().ToString();
fileName = Path.ChangeExtension(fileName, extension);
return await storageFolder.CreateFileAsync(fileName, CreationCollisionOption.ReplaceExisting);

Choose a reason for hiding this comment

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

why not assign path here and then ditch the else{} wrapper on the next line?

@gre
Copy link
Owner

gre commented Mar 8, 2017

incredible PR. How awesome is that!
Also thanks for reviewing it @matthargett .

@gre
Copy link
Owner

gre commented Mar 8, 2017

@ryanlntn is the PR ready to be merged now? :)

@ryanlntn
Copy link
Contributor Author

ryanlntn commented Mar 8, 2017

@gre Yes but you might want to hold off until microsoft/react-native-windows#1050 is merged since it won't work without that.

@gre
Copy link
Owner

gre commented Mar 8, 2017

@ryanlntn ok nice!

BTW if you want to have more testing, maybe it would be great to make that https://github.com/gre/react-native-view-shot-example a windows app too. it's basically an app I use to test the library and it should cover all different options / different kind of views.

@ryanlntn
Copy link
Contributor Author

ryanlntn commented Mar 8, 2017

@gre Planning on it! 😄 I'm also working on integrating it into https://github.com/Galooshi/happo

@gre
Copy link
Owner

gre commented Mar 8, 2017

It's funny you had to PR in RN the same third party UIBlock feature for the same limitation hehe.
also the windows impl looks like similar to the Android version hey ? :)

@ryanlntn
Copy link
Contributor Author

ryanlntn commented Mar 9, 2017

@gre Yeah a blatant copy actually. 😛

matthargett pushed a commit to microsoft/react-native-windows that referenced this pull request Mar 9, 2017
* This will allow gre/react-native-view-shot#45 (and in turn, Galooshi/happo ) to allow us to do screenshot-based tests in the CI
@ryanlntn
Copy link
Contributor Author

@gre microsoft/react-native-windows#1050 has landed in RNW master.

@gre
Copy link
Owner

gre commented Mar 11, 2017

so cool! I'll probably get this merged anyway :) thanks again!

@gre gre self-assigned this Mar 11, 2017
@gre gre merged commit e406cb8 into gre:master Mar 11, 2017
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