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

Fix "WPF- OxyPlot doesn't render inside a Popup" (#1796) #1797

Merged
merged 1 commit into from
Oct 14, 2021

Conversation

Tetedeiench
Copy link
Contributor

@Tetedeiench Tetedeiench commented Oct 5, 2021

Fix "WPF- OxyPlot doesn't render inside a Popup" (#1796)

Fixes #1796 .

Checklist

  • I have included examples or tests
  • I have updated the change log
  • I am listed in the CONTRIBUTORS file
  • I have cleaned up the commit history (use rebase and squash)

Changes proposed in this pull request:

@oxyplot/admins

update changelog

Fix "WPF- OxyPlot doesn't render inside a Popup" (oxyplot#1796)
Copy link
Contributor

@VisualMelon VisualMelon left a comment

Choose a reason for hiding this comment

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

I've tested and the fix looks good to me.

We probably need an example (so that it's easier to test in future); I put a simple example together in https://github.com/VisualMelon/oxyplot/tree/bugfix-wpf-popup which we could use, but if you have a better idea for a use-case that involves popups that would be welcome!

@@ -1,6 +1,9 @@
# Change Log
All notable changes to this project will be documented in this file.

### Fixed
Copy link
Contributor

Choose a reason for hiding this comment

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

Should ideally have the unrelease/added/changed/fixed/removed sections like in https://raw.githubusercontent.com/oxyplot/oxyplot/StemSeriesActualMarker/CHANGELOG.md , but we can do that separately

@VisualMelon
Copy link
Contributor

(Note to self: need to check the Skia stuff as well)

@VisualMelon
Copy link
Contributor

VisualMelon commented Oct 6, 2021

Fixed Skia stuff in my branch, and added a pointless/fun popup pre-view for the example browser (so we can accidently test popups and Skia scaling at the same time, all the time)

Copy link
Contributor

@VisualMelon VisualMelon left a comment

Choose a reason for hiding this comment

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

I think I'm happy to merge this; need separate PRs for:

  • Skia Fix
  • Example of some sort...
  • Re-start changelog

Will leave it a little longer for the other maintainers to take a look.

@Tetedeiench
Copy link
Contributor Author

I'm happy my PR was useful :) Sorry for the changelog, did my best.

Adding a popup in the examples is a great idea 👍

@VisualMelon
Copy link
Contributor

Just to keep you in the loop, I'm somewhat indisposed at the moment, and don't want to hit merge until I have the energy to follow up with the other bits properly. Hopefully I'll be back in action some time in the next few days.

@Jonarw
Copy link
Member

Jonarw commented Oct 10, 2021

Looks good to me, no objections to merging this.

Fixed Skia stuff in my branch, and added a pointless/fun popup pre-view for the example browser (so we can accidently test popups and Skia scaling at the same time, all the time)

Sounds great!

I am wondering: The code until now assumed that at the root of the visual tree is always a Window, which obviously isn't true, because it can also be a PopupRoot. Are there any other possibilities for the root of the visual tree besides these two that we'd want to support?

@AELIENUS
Copy link
Contributor

I am currently testing a solution for ElementHosts with the code provided in this PR. The only addition to this would be to also match to AdornerDecorators in IsInVisualTree. Although i have to test this a little be more. Please check #1800 for reference.

@VisualMelon VisualMelon merged commit e384904 into oxyplot:develop Oct 14, 2021
@VisualMelon
Copy link
Contributor

@AELIENUS thanks for following that up!

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.

WPF- OxyPlot doesn't render inside a Popup (fix included)
4 participants