-
Notifications
You must be signed in to change notification settings - Fork 730
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
Android implementation for Windows.Data.Pdf #1796
Conversation
update from source
update from source
update PdfDocument-branch from source
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Very nice PR, thank you for your contribution !
A few things:
- You'll need to update the release notes
- You'll need to add a article that explains the available features and limitations of the current implementation (see https://github.com/unoplatform/uno/tree/master/doc/articles/features).
- A UI Test is important for that, and it may be an easy one to do. Embed a PDF file in the sample app, make it render and compare the resulting render with a master file (see https://github.com/unoplatform/uno/blob/master/doc/articles/uno-development/working-with-the-samples-apps.md#creating-ui-tests).
@@ -0,0 +1,81 @@ | |||
#if __ANDROID__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file should be named ".android.cs", so that the globbing picks it up:
uno/src/PlatformItemGroups.props
Line 70 in a6db376
<None Include="**\*.Android.cs" Exclude="bin\**\*.Android.cs;obj\**\*.Android.cs" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sorry, I used ".Droid.cs" naming convention from here
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I made a PR to fix the docs
|
||
namespace Windows.Data.Pdf | ||
{ | ||
public partial class PdfDocument : IDisposable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please make sure that the .editorconfig file is used, so that it aligns with the global whitespace configuration.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, I applied Code Cleanup, but for some reason it removes whitespace before #if
conditions
pdfRenderer.Dispose(); | ||
} | ||
|
||
private readonly PdfRenderer pdfRenderer; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please place fields at the top of the file
{ | ||
throw new ArgumentNullException(nameof(options)); | ||
} | ||
if (options.DestinationWidth > int.MaxValue) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
use braces for single line conditions. (See .editorconfig file)
pdfPage.Dispose(); | ||
} | ||
|
||
private static RectF AsAndroidRectF(Rect rect) => new RectF( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Move this here:
uno/src/Uno.Foundation/Rect.Android.cs
Line 10 in a6db376
public partial struct Rect |
src/Uno.UWP/Uno.csproj
Outdated
@@ -38,6 +38,15 @@ | |||
<ItemGroup> | |||
<UpToDateCheckInput Include="**\*.cs" Exclude="bin\**\*.cs;obj\**\*.cs;" /> | |||
</ItemGroup> | |||
|
|||
<ItemGroup> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please remove, caused by an android issue.
{ | ||
return wrapper.GetSourceStream(); | ||
} | ||
else throw new ArgumentException($"stream must be a {nameof(RandomAccessStreamWrapper)}", nameof(stream)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Braces even for single line conditions
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added to docs PR over at https://github.com/unoplatform/uno/pull/1212/files#diff-336e4bcde45e4393a322cd34f381311dR35
Fixed issues in code, but CI build still fails |
@artemious7 are you able to rebase this PR to resolve the conflicts and check the build errors please? |
@artemious7 Hi, can you still get to this PR to bring it up to date please? |
@MartinZikmund, no, sorry, I don't have the time recently and I don't use this code. |
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
Inport initial implementation from PR unoplatform#1796
GitHub Issue (If applicable): #1629
PR Type
What kind of change does this PR introduce?
What is the current behavior?
No implementation for Windows.Data.Pdf
What is the new behavior?
Stream
toIRandomAccessStream
and vice versa (which is not a real implementation, but just a stub to passStream
to methods accepting onlyIRandomAccessStream
).PR Checklist
Please check if your PR fulfills the following requirements:
(not sure what current supported SDKs means, but I have tested it on my Android project)
Screenshots Compare Test Run
results.Other information
Internal Issue (If applicable):