Skip to content

Conversation

@TheMountain427
Copy link

Note: Closed previous PR since I forgot to switch my git config user off of my company account

New Feature: FitToScreen

Description

Add 'Bring into view feature' from the roadmap. This will fit the Viewport to the center point of all items, fitting all items in screen if the MinZoom and MaxZoom allow it, otherwise it will place the Viewport in the center of items at max/min zoom.

Changes

  • Added FitToScreen method.
  • Added Gesture/Command for the FitToScreen feature. Default is Key.Home

Testing

  • I Tested this using the existing testing app. All of the following passed when I manually tested them.
    • To test this, add a bunch of shapes onto the canvas using existing the controls. Then press the Home key.
    • Test with multiple items close together => Viewport respects MinZoom
    • Test with multiple items very far apart => Viewport respects MaxZoom
    • Test with no items => Does nothing. This could be changed to just set viewport location to (0,0) if desired. See the ItemsHost.Children.Count check
    • Test with items off screen => Viewport fits those items to screen
    • Test with a large amount of objects => No performance impact

Notes

  • I am unfamiliar with the testing suite and was unable to figure out exactly how to add a new automation test.
  • Since the ViewportZoom property already coerces the zoom value, I did not add checks on whether the new zoom is within the set MinZoom and MaxZoom values. If that coercing function is removed, this will need to be updated.
  • The FitToScreen method includes optional marginX and marginY parameters, this can be used to add a margin when fitting the screen. This can be made a DependencyProperty if desired, but as of now it defaults to 0.
  • I added this as a seperate partial class file as the functionality did not fit the existing Zooming partial class file, since the function does both zooming and panning. Addtionally, the other roadmap item for bringing a container into view would most likely want to be grouped with this one.

Potential Issues

  • This feature is a little wonky when scroll bars are present. To see this, move an item outside of the Viewport, then hit the Home key. The screen will appear to fit correctly. Hit the Home key again. The viewport will adjust an additional time.
    • This is due to the visible scrollbars affecting the viewport size. When the first fit is made, the viewport is ViewportSize - ScrollBarWidth. After the first fit, the scrollbars are no longer visible and the viewport has actually increased by the width of the scrollbars.
    • I added a comment in the code describing this, but decided it was not a major issue as the first FItToScreen is close enough.
    • The comment for reference:
                // If the canvas has a ScrollViewer and the scrollbars are visible,
              // the offset will be off by a tad. Calling this command immediately again will fix it.
              // We can check if there are scrollbars present by doing:
              //      if (ExtentHeight != ViewportSize.Height) => VerticalScrollBar present
              //      if (ExtentWidth != ViewportSize.Width) => HorizontalScrollBar present
              // You can then try to use SystemParameters.ScrollWidth to try and account for it,
              // but I could not get the offset perfect. Additionally, if the ScrollBar is ever styled,
              // then we would have to go find the ScrollBar and get its width. But that opens up another whole
              // can: we would want to cache it so we don't traverse the tree everytime, but then we need to
              // add handling for if/when the styling changes.
    
              // Thus I have decided that being 10 pixels off is not the end of the world, and if anyone is
              // bothered they can hit the FitToScreen button again.
    
 

@mircea21S
Copy link
Owner

Hey!

Appreciate the PR and the help. Just want to let you know that I've seen this, but I was caught up with life lately and didn't find the time to go through this, so once I am able to thoroughly go through this I will respond to you, no worries.

Thank you!

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.

2 participants