Add lazy loading threshold for ImageEx control - #3483
Conversation
merge original source
merge original source
merge original source
|
Thanks h82258652 for opening a Pull Request! The reviewers will test the PR and highlight if there is any conflict or changes required. If the PR is approved we will proceed to merge the pull request 🙌 |
ghost
added
the
|
@michael-hawker Done, can you review again? |
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Couple of questions. Also, does this mean we can close #3255 or are these independent?
| LockObj = new object(); | ||
|
|
||
| EffectiveViewportChanged += ImageExBase_EffectiveViewportChanged; | ||
| LayoutUpdated += ImageExBase_LayoutUpdated; |
There was a problem hiding this comment.
Don't we need to do both still?
There was a problem hiding this comment.
@michael-hawker Don't need, issue #3258 is occurred by register EffectiveViewportChanged while the control host in a Viewbox, it is a framework bug.
| <Setter Property="Background" Value="Transparent" /> | ||
| <Setter Property="Foreground" Value="{ThemeResource ApplicationForegroundThemeBrush}" /> | ||
| <Setter Property="IsTabStop" Value="False" /> | ||
| <Setter Property="LazyLoadingThreshold" Value="300" /> |
There was a problem hiding this comment.
Is there a reason you picked 300?
There was a problem hiding this comment.
@michael-hawker About this I think a lot. I searched google and it tells me Chromium will calculate this by the screen size and network type. And then I searched the most widely used JavaScript image lazy loading library, got this. This lib use 300px as default. I think UWP is mostly used on a PC, not a mobile device. The network should be not bad, 300px should be suitable for most situations.
|
Hello @michael-hawker! Because this pull request has the p.s. you can customize the way I help with merging this pull request, such as holding this pull request until a specific person approves. Simply @mention me (
|
michael-hawker
left a comment
michael-hawker
left a comment
There was a problem hiding this comment.
Caught this one last thing I think. Then we should be good.
|
This PR has been marked as "needs attention 👋" and awaiting a response from the team. |
5 participants
Fixes
Add lazy loading threshold for
ImageExcontrol and change the lazy loading implement that will be also fixed #3258 .PR Type
What kind of change does this PR introduce?
What is the current behavior?
If enable lazy loading, only the ImageEx entered the viewport, the source will start to load.
What is the new behavior?
Give a threshold for triggering the lazy loading behavior, the default value is 300 px.
PR Checklist
Please check if your PR fulfills the following requirements:
Other information