Skip to content
This repository was archived by the owner on May 15, 2024. It is now read-only.

Screenshots #744 -> #613#1356

Merged
mattleibow merged 68 commits intodev/add-media-pickerfrom
dev/screenshots
Aug 25, 2020
Merged

Screenshots #744 -> #613#1356
mattleibow merged 68 commits intodev/add-media-pickerfrom
dev/screenshots

Conversation

@mattleibow
Copy link
Copy Markdown
Member

Description of Change

Merge the #744 PR into the #613 PR to see what things are needed to change to merge the result types.

Bugs Fixed

API Changes

Behavioral Changes

New screenshot API.

PR Checklist

  • Has tests (if omitted, state reason in description)
  • Has samples (if omitted, state reason in description)
  • Rebased on top of master at time of PR
  • Changes adhere to coding standard
  • Updated documentation (see walkthrough)

Niklas Schilli and others added 30 commits March 19, 2019 23:06
…same media file as in other struct

# Conflicts:
#	Xamarin.Essentials/Platform/Platform.android.cs
Context: https://github.com/dotnet/core/blob/79c19c12ab2bc11912551bb0c3025f602cd541d9/Documentation/diagnostics/portable_pdb.md

In doing some performance testing with builds in VS 2019, I noticed:

    115 ms  _ConvertPdbFiles                           1 calls

And looking at files were converted:

    ConvertDebuggingFiles
        Parameters
            Files
                C:\ProgramData\Xamarin\NuGet\xamarin.essentials\1.0.1\lib\monoandroid81\Xamarin.Essentials.pdb
        [Output] ConvertedFiles:
            C:\ProgramData\Xamarin\NuGet\xamarin.essentials\1.0.1\lib\monoandroid81\Xamarin.Essentials.dll

The Xamarin.Essentials NuGet package is shipping a non-portable PDB
file. This is a Windows-specific format that Mono doesn't support.

Xamarin.Android runs a tool called `pdb2mdb` when it encounters a
non-portable PDB file so symbols be converted to something that Mono
can use. If you have `DebugType=full` or `DebugType=pdbonly`,
Xamarin.Android has to do this extra work to convert it.

I see no drawbacks to just use `DebugType=portable` in this project
all the time?

Since Xamarin.Essentials uses sourcelink, I double-checked and they
support portable PDBs:

https://github.com/dotnet/sourcelink#prerequisites-for-net-projects

I sent a similar PR to Xamarin.Forms, shipping since 3.4. I wrote a
bit more detail on symbol files there if you need more info:

xamarin/Xamarin.Forms#4201
…ic cache director and use correct path for file provider. (#789)
* Fixes #745 to return real metrics, not app metrics on display.

* Add using statements to cleanup displays and metrics
* Check for null MainView

* Use the ActionSendMultiple on android for multi-attachments
* Implement Open File in Launcher.  Fixed #601 & #425

* Add title to open file request for android launcher

* Clean FileBase and AttachmentName. Added documentation.

* Fix encoding

* Remove namesapces that aren' t needed
Copy link
Copy Markdown
Contributor

@pictos pictos left a comment

Choose a reason for hiding this comment

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

I like it, I just point some places that, maybe we can use ConfigureAwait(false).

Comment thread Xamarin.Essentials/Screenshot/Screenshot.android.cs Outdated
Comment thread Xamarin.Essentials/Screenshot/Screenshot.uwp.cs Outdated
Comment on lines +8 to +9
public static bool IsCaptureAvailable
=> PlatformCanCapture;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Not sure what we want to do here...

I see the media picker has a public property, but the app actions does not.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Just as a heads up, as the audio recording pr is quite similar with regards to public api. There exists a similar property on that branch, CanRecordAudio. IsFeatureSupported is probably more universally suited for this?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Thanks for that. I think we might be going for an Is<operation>Available naming convention. But, I need to confirm with @Redth and @jamesmontemagno what this will be.

`bmp` was inaccessible in some cases at this point and getting the `PixelWidth` was throwing.
@mattleibow mattleibow marked this pull request as ready for review August 25, 2020 14:56
@mattleibow mattleibow merged commit 5e7d533 into dev/add-media-picker Aug 25, 2020
@mattleibow mattleibow deleted the dev/screenshots branch August 25, 2020 14:56
@Redth Redth mentioned this pull request Sep 1, 2020
5 tasks
@jamesmontemagno jamesmontemagno added this to the 1.6.0 milestone Sep 23, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.