Skip to content
This repository has been archived by the owner on May 10, 2023. It is now read-only.

fix: prevent horizontal scrollbar from flickering during swipe #467

Merged

Conversation

olejorgenb
Copy link
Contributor

A minor issue, but when reviewing sentences the TinderCard animation will make the horizontal (and sometimes the vertical scrollbar) appear. This is quite jarring to me.

The TinderCard library recommend using overflow: hidden outright, but we need to allow vertical scrolling. I couldn't find a relatively simple solution for the vertical scrollbar.

But testing most pages I could not find any which need horizontal scrolling - so using overflow-x: hidden should be ok atm. Though it feels a bit dirty to apply the rule to all pages - but making it only apply to the review page isn't straight forward (applying it to the form-div doesn't work due to the padding)

Personally I would prefer no animation on desktop, but that's another matter. (and easily solvable using a user-stylesheet)

/* https://commonvoice.mozilla.org/sentence-collector/#/review */
body {
    overflow: hidden;
}

section.cards-container > div {
    transition: none !important;
}

When reviewing sentences the TinderCard animation will make the horizontal (and
sometimes the vertical scrollbar) appear. This is quite jarring to me.

The TinderCard library actually recommend using `overflow: hidden`, but we need
to allow vertical scrolling.

As far as I could see no of the other pages need horizontal scrolling though -
so we can at least fix one of the cases.
Copy link
Member

@MichaelKohler MichaelKohler left a comment

Choose a reason for hiding this comment

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

Thanks! While this is definitely not the most perfect solution, I'm fine with this. I heavily doubt we're ever gonna have a case where we want the body to overflow horizontally.

@MichaelKohler
Copy link
Member

MichaelKohler commented Aug 7, 2021

Personally I would prefer no animation on desktop, but that's another matter. (and easily solvable using a user-stylesheet)

I quickly tested this with your snippet, and I'd say the swipe movement transition is probably enough, I don't think we need to have them fly further than just that. Additionally to that, this also completely disables the animation when using the buttons, which I think would be a very good thing. I think I would approve a PR implementing that :)

@MichaelKohler MichaelKohler merged commit e8ff7d5 into common-voice:main Aug 7, 2021
@MichaelKohler
Copy link
Member

@olejorgenb I haven't created an issue for this, but you might be interested in fixing this (obviously only if you want to!):

If the full page doesn't fit in the viewport and you have to scroll down a bit, when using the buttons, mostly skip and approve, sometimes the viewport jumps up to the top of the page again. My guess would be that this is related to the transitions and possibly is a bug somewhere in the library, but haven't had time to investigate further. Would you like to have a look?

@olejorgenb
Copy link
Contributor Author

Couldn't reproduce here 🤔 Unlikely to have time in the near future anyway :)

MichaelKohler pushed a commit that referenced this pull request Aug 9, 2021
# [2.9.0](v2.8.1...v2.9.0) (2021-08-09)

### Bug Fixes

* add Nyankole (fixes [#468](#468)) ([a3eb3ff](a3eb3ff))
* do not violate rule of hooks ([9dc159d](9dc159d))
* prevent horizontal scrollbar from flickering during swipe ([#467](#467)) ([e8ff7d5](e8ff7d5))
* remove certain zh-CN sentences ([9743fbe](9743fbe))

### Features

* request confirmation when leaving a form with unsubmitted data ([#465](#465)) ([e117831](e117831))
@MichaelKohler
Copy link
Member

🎉 This PR is included in version 2.9.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants