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

Fixes #745 to return real metrics, not app metrics on display.#749

Merged
jamesmontemagno merged 6 commits intomasterfrom
dev/gh-745
May 30, 2019
Merged

Fixes #745 to return real metrics, not app metrics on display.#749
jamesmontemagno merged 6 commits intomasterfrom
dev/gh-745

Conversation

@jamesmontemagno
Copy link
Copy Markdown
Collaborator

Description of Change

Uses real device metrics instead of app.

Bugs Fixed

Provide links to issues here. Ensure that a GitHub issue was created for your feature or bug fix before sending PR.

API Changes

Behavioral Changes

Will return different device metrics, but are correct to match iOS/UWP

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)

@jamesmontemagno jamesmontemagno added the awaiting-review This PR needs to have a set of eyes on it label Mar 26, 2019
@jamesmontemagno jamesmontemagno added this to the 1.1.1 milestone Mar 26, 2019
@ghost
Copy link
Copy Markdown

ghost commented Mar 26, 2019

OPS Build status updates of commit ca36046:

✅ Validation status: passed

File Status Preview URL Details
Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@ghost
Copy link
Copy Markdown

ghost commented Apr 27, 2019

Docs Build status updates of commit 3c63626:

✅ Validation status: passed

File Status Preview URL Details
Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

newky2k
newky2k previously approved these changes May 28, 2019
mattleibow
mattleibow previously approved these changes May 29, 2019
static DisplayInfo GetMainDisplayInfo()
{
var displayMetrics = Platform.AppContext.Resources?.DisplayMetrics;
var displayMetrics = new DisplayMetrics();
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It might be better to wrap this in a using to dispose of the object sooner. Not sure how big this object is.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed up :) let me know what you think.

@newky2k newky2k added DO-NOT-MERGE Don't merge it.... don't do it! needs-more-discussion This needs more discussion or info before discussing it as a proposal. and removed awaiting-review This PR needs to have a set of eyes on it labels May 29, 2019
@jamesmontemagno jamesmontemagno dismissed stale reviews from mattleibow and newky2k via e8f62f0 May 30, 2019 00:49
@ghost
Copy link
Copy Markdown

ghost commented May 30, 2019

Docs Build status updates of commit e8f62f0:

✅ Validation status: passed

File Status Preview URL Details
Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@ghost
Copy link
Copy Markdown

ghost commented May 30, 2019

Docs Build status updates of commit 5baf369:

✅ Validation status: passed

File Status Preview URL Details
Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

Copy link
Copy Markdown
Contributor

@newky2k newky2k left a comment

Choose a reason for hiding this comment

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

Tested and builds and runs on real device

@newky2k newky2k added ready-to-merge Review completed, Ready for API review and merge and removed DO-NOT-MERGE Don't merge it.... don't do it! needs-more-discussion This needs more discussion or info before discussing it as a proposal. labels May 30, 2019
@ghost
Copy link
Copy Markdown

ghost commented May 30, 2019

Docs Build status updates of commit 0a12af0:

✅ Validation status: passed

File Status Preview URL Details
Xamarin.Essentials/DeviceDisplay/DeviceDisplay.android.cs ✅Succeeded

For more details, please refer to the build report.

Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.

@jamesmontemagno jamesmontemagno merged commit 63fb146 into master May 30, 2019
@jamesmontemagno jamesmontemagno deleted the dev/gh-745 branch May 30, 2019 23:45
@jamesmontemagno jamesmontemagno removed this from the 1.1.1 milestone Jun 18, 2019
@jamesmontemagno jamesmontemagno added this to the 1.2.0 milestone Jun 18, 2019
Mrnikbobjeff pushed a commit to Mrnikbobjeff/Essentials that referenced this pull request Aug 28, 2019
…xamarin#749)

* Fixes xamarin#745 to return real metrics, not app metrics on display.

* Add using statements to cleanup displays and metrics
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

ready-to-merge Review completed, Ready for API review and merge

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants