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 column swiping #4211

Merged
merged 1 commit into from Jul 15, 2017
Merged

Fix column swiping #4211

merged 1 commit into from Jul 15, 2017

Conversation

unarist
Copy link
Contributor

@unarist unarist commented Jul 15, 2017

This fixes broken behavior and enable animation only on swiping.

  • onTransitionEnd called before onChangeIndex if animateTransitions is false. In a result, location.pathname won't be updated on swiping.
  • Transition depends animateTransiton value which is set before transition, i.e. we need to maintain true as much as possible to animate swiping.

This also removes animation from transition caused by location changing.

Before

before

After

after

cc @sorin-davidoi

This fixes broken behavior and enable animation only on swiping.
@unarist unarist added bug Something isn't working ui Front-end, design labels Jul 15, 2017
@Gargron Gargron merged commit 6954397 into mastodon:master Jul 15, 2017

const columnIndex = getIndex(this.context.router.history.location.pathname);
const shouldAnimate = Math.abs(this.lastIndex - columnIndex) === 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't we animate if we move to the previous / next tab? Might help with discoverability of the feature and - subjective - I think it looks nice.

Copy link
Contributor

Choose a reason for hiding this comment

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

I also feel that it improves perceived performance.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When we use animation for location change based transition, columnIndex is already new one before transition, whereas it's old one on swiping based transition. So destination column will be rendered before animated transition.

I don't feel it improves perceived performance so much, on my (slow) device which takes some freeze time anyway. Also transition to rendering looks not bad, but different from behavior on swiping.

Well...I wonder that if we make same behavior as swiping, i.e. render destination while/after transition.

@unarist unarist deleted the fix-swipe branch September 30, 2018 14:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working ui Front-end, design
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants