Skip to content

SlidableListItem improvements. #241

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 14 commits into from
Sep 14, 2016
Merged

SlidableListItem improvements. #241

merged 14 commits into from
Sep 14, 2016

Conversation

bkaankose
Copy link
Contributor

@msftclas
Copy link

Hi @bkaankose, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!

The agreement was validated by Microsoft and real humans are currently evaluating your PR.

TTYL, MSBOT;

@deltakosh
Copy link
Contributor

Pinging @nmetulev for validation :)

@deltakosh
Copy link
Contributor

While you're on this one, do you mind considering this one: #236?

@bkaankose
Copy link
Contributor Author

@deltakosh will definitely look into that.

@deltakosh
Copy link
Contributor

Thanks a lot!

@bkaankose
Merge remote-tracking branch 'refs/remotes/Microsoft/dev' into devnext
729a0f9
@bkaankose
Merge remote-tracking branch 'refs/remotes/Microsoft/dev' into devnext
6ec3d3b
@bkaankose
Fixes #236
5f1599b 
* IsOffsetLimited property added.

When IsOffsetLimited is true users can only swipe ListViewItem until the
transformation (left / right panel width + 24 px).
deltakosh
deltakosh reviewed Aug 30, 2016
View reviewed changes
Microsoft.Toolkit.Uwp.UI.Controls/SlidableListItem/SlidableListItem.cs
_leftCommandPanel.Opacity = 1;
_rightCommandPanel.Opacity = 0;

if (abs < ActivationWidth)
Copy link
Contributor

Choose a reason for hiding this comment

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

why removing ActivationWidth?

Copy link
Contributor Author

@bkaankose bkaankose Aug 30, 2016 •
edited
Loading

Choose a reason for hiding this comment

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

Actually it was something that I deleted for debug purposes. If I add that, there has to be more work onto this commit because it's messing with the content that users see.

I will update it soon, thanks for pointing out.

ScottIsAFool
ScottIsAFool reviewed Sep 2, 2016
View reviewed changes
Microsoft.Toolkit.Uwp.UI.Controls/SlidableListItem/SlidableListItem.cs Outdated
{
if (_commandContainer != null)
if (IsRightSwipeEnabled && e.Delta.Translation.X > 0)
{ // Swiping from left to right.
Copy link
Contributor

Choose a reason for hiding this comment

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

Comment should be on a new line.

deltakosh
deltakosh reviewed Sep 6, 2016
View reviewed changes
Microsoft.Toolkit.Uwp.UI.Controls/SlidableListItem/SlidableListItem.cs Outdated

_leftCommandPanel.Opacity = 1;
_rightCommandPanel.Opacity = 0;
var swipeThreshold = _leftCommandPanel.ActualWidth + 24;
Copy link
Contributor

Choose a reason for hiding this comment

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

We should be able to set to value (24)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you mean allowing developers to change extra swipethreshold?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep:)

@deltakosh
Copy link
Contributor

ping?

@bkaankose
Copy link
Contributor Author

Will send an update in a few hours.

@deltakosh
Copy link
Contributor

Woot that was fast :D

@bkaankose
Copy link
Contributor Author

I think that Gitter thing sounds perfect for these kind of things :D

@deltakosh
Copy link
Contributor

Lol....duly noted;)

@bkaankose
Copy link
Contributor Author

pinging @deltakosh for validation. looking good?

@deltakosh
Copy link
Contributor

LGTM!

@deltakosh
Copy link
Contributor

Did you updated the doc?

@bkaankose
Copy link
Contributor Author

No I didn't yet. I will asap.

@bkaankose bkaankose mentioned this pull request Sep 13, 2016
Merged
@deltakosh
Copy link
Contributor

Waiting for another reviewer

@deltakosh
Copy link
Contributor

Merging this one. @reviewers: Please create an issue if problems found

@deltakosh deltakosh merged commit 547cf90 into CommunityToolkit:dev Sep 14, 2016
@bkaankose bkaankose deleted the devnext branch September 14, 2016 19:51
@nmetulev nmetulev mentioned this pull request Oct 1, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@bkaankose @msftclas @deltakosh @ScottIsAFool