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

FancyZones editor magnetic snapping effect #1277

Closed
wants to merge 38 commits into from

Conversation

ivan100sic
Copy link
Contributor

Implemented a solution to Issue #585: FancyZones: Alignment/Snapping/Ruler

Summary of the Pull Request

This PR implements one aspect of Issue #585. Now, windows in the Canvas zone editor will snap to each other when being moved and resized.

References

PR Checklist

  • [] Applies to FancyZones: Alignment/Snapping/Ruler #585
  • [] CLA signed. If not, go over here and sign the CLA
  • [] Tests added/passed
  • [] Requires documentation to be updated
  • [] I've discussed this with core contributors already. If not checked, I'm ready to accept this work might be rejected in favor of a different grand plan. Issue number where discussion took place: #xxx

Detailed Description of the Pull Request / Additional comments

There are two implementations of snapping available, but only one is being used.

Validation Steps Performed

Manually tested the Canvas zone editor in various scenarios.

@ivan100sic ivan100sic changed the title Finished implementing the magnetic snapping effect FancyZones editor magnetic snapping effect Feb 12, 2020
@crutkas
Copy link
Member

crutkas commented Feb 12, 2020

let’s hold on merging until 0.15 ships please

@enricogior
Copy link
Contributor

Yes, sure.

ivan100sic and others added 10 commits March 2, 2020 11:34
* Getting ready for v0.15 update for readme.

* getting ready for 0.15

* spelling tweak

* filled in update section

* fixed spelling mistakes

* updating to what POR is

* Update README.md

* Update README.md

* adding back in MSI

* getting readme ready for 0.15

* tweaks

* adding oss to oss links

* fixing links

* tweaking file names

* Update README.md

* Update README.md

* Update README.md

fixing typo

* Update README.md
* Fix migrating never applied custom zonesets

* Handle json parsing exception in LoadCustomModels()
* adding preview tag for desktop and start menu

* getting to load, code from wixtoolset/issues#5700 (comment)
@crutkas crutkas removed this from the 20.02 release milestone Mar 4, 2020
ivan100sic and others added 4 commits March 5, 2020 10:02
* Fix PowerRename UI doesn't handling DPI changes

* Address PR comments

* Address PR comments
Fixed an issue (microsoft#365) where a window gets resized after it gets dropped into a zone.
}

private void SWResize_DragDelta(object sender, System.Windows.Controls.Primitives.DragDeltaEventArgs e)
private Settings _settings = ((App)Application.Current).ZoneSettings;
Copy link
Contributor

Choose a reason for hiding this comment

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

_settings is not used anymore, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, that can safely be removed.

Comment on lines 59 to 63
/// Just pass it the canvas arguments. Use mode
/// to tell it which edges of the existing masks to use when building its list
/// of snap points, and generally which edges to track. There will be two
/// SnappyHelpers, one for X-coordinates and one for
/// Y-coordinates, they work independently but share the same logic.
Copy link
Contributor

Choose a reason for hiding this comment

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

Better formatting:

            ///     Just pass it the canvas arguments. Use mode to tell it which edges
            ///     of the existing masks to use when building its list of snap points,
            ///     and generally which edges to track. There will be two SnappyHelpers,
            ///     one for X-coordinates and one for Y-coordinates, they work
            ///     independently but share the same logic.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I'll unindent these 5 lines.

@enricogior
Copy link
Contributor

@ivan100sic
when a custom layout is created, it's permanently bound to the current screen size. Changing the monitor scaling will result in the layout to not being able to use the increased screen size, or to go out of bound if the new screen size is smaller.

Comment on lines 32 to 34
ModeLow = 1,
ModeHigh = 2,
ModeBoth = 3,
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the enum already states that this is the ResizeMode, I would suggest to rename the values to:

  • BottomEdge
  • TopEdge
  • BothEdges

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Perhaps LowEdge, HighEdge, BothEdges?

Copy link
Contributor

Choose a reason for hiding this comment

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

Top and Bottom are better terms in this case.

Copy link
Contributor

Choose a reason for hiding this comment

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

Or are these referring to the Y axis as well?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

They are used in both X and Y axes.

Copy link
Contributor

Choose a reason for hiding this comment

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

In case of Y, top edge is the left edge, correct?

@ivan100sic
Copy link
Contributor Author

I messed up the history on this branch, so I'm closing this pull request in favor of #1503

@ivan100sic ivan100sic closed this Mar 9, 2020
@ivan100sic ivan100sic deleted the snapping-improved-new branch June 8, 2020 10:09
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.

8 participants