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.

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

Unify tab transitions with the other transitions #1332

Merged
merged 2 commits into from
Nov 9, 2023
Merged

Unify tab transitions with the other transitions #1332

merged 2 commits into from
Nov 9, 2023

Conversation

axpoems
Copy link
Contributor

@axpoems axpoems commented Nov 7, 2023

content_slide3

Resolve #1328

HenrikJannsen
HenrikJannsen approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

ACK

The transition used for the content views (Transitions.transitContentViews) has DEFAULT_DURATION/2 duration. I think we should adjust that as well to have both the same. The DEFAULT_DURATION/4 seems better to me.

@axpoems
Copy link
Contributor Author

axpoems commented Nov 8, 2023

Added the change. @HenrikJannsen were you referring to fadeIn transition?

HenrikJannsen
HenrikJannsen approved these changes Nov 8, 2023
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

@HenrikJannsen
Copy link
Contributor

Added the change. @HenrikJannsen were you referring to fadeIn transition?

Both the fade and the side animation.

@axpoems axpoems force-pushed the unify-tab-transitions branch from e6c6947 to a65df23 Compare November 8, 2023 19:07
@axpoems
Copy link
Contributor Author

axpoems commented Nov 8, 2023

Both the fade and the side animation.

Added it for the slide out transition as well. Can you confirm those are the ones you were referring?

HenrikJannsen
HenrikJannsen approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@HenrikJannsen HenrikJannsen left a comment

Choose a reason for hiding this comment

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

utACK

djing-chan
djing-chan approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@djing-chan djing-chan left a comment

Choose a reason for hiding this comment

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

ACK

The trade wizard/ create offer popups use a different transition style (slower, left/right). The other popups with tabs (reputation) use the also the fast right out transition. I think we should harmonize all those transitions (not as part of that PR) and use the same methods in the Transitions class. I find the left/right transition when using back/next buttons more intuitive. Not sure if that is also the best option for tab views without next/back buttons.

alvasw
alvasw approved these changes Nov 9, 2023
View reviewed changes
Copy link
Contributor

@alvasw alvasw left a comment

Choose a reason for hiding this comment

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

utACK

@alvasw alvasw merged commit 813e02f into bisq-network:main Nov 9, 2023
12 checks passed
@axpoems axpoems mentioned this pull request Nov 21, 2023
Closed
@axpoems axpoems deleted the unify-tab-transitions branch December 30, 2023 07:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@djing-chan djing-chan djing-chan approved these changes

@HenrikJannsen HenrikJannsen HenrikJannsen approved these changes

@alvasw alvasw alvasw approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Harmonize tab transitions to other transitions
4 participants
@axpoems @HenrikJannsen @alvasw @djing-chan