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 jittering in emby-checkbox #4929

Merged
merged 1 commit into from
Nov 3, 2023
Merged

Conversation

rafma0
Copy link
Contributor

@rafma0 rafma0 commented Nov 2, 2023

I've noticed that when you navigate through checkboxes or toggle them from the TV UI, there's this strange blinking animation. While looking for the culprit, I stumbled upon forceRefresh, and it looks like it's been around since the repo's creation.

I've been trying to figure out why it's there or look for any other references but came up with nothing. Removing it seems to solve the problem on my Tizen TV.

I initially recorded this issue in Chrome on my PC by spoofing the Tizen user agent, but I also did a test run on my TV using jellyfin-tizen. The TV didn't explode, so I thought it’d be a good idea to create this PR. :)

current behaviour
https://github.com/jellyfin/jellyfin-web/assets/28578847/21760c3f-4f94-4d24-826b-91a4c0892368

removing the hack
https://github.com/jellyfin/jellyfin-web/assets/28578847/9ff35bd6-bf4d-4677-ac6a-f92ef63c55c6

@rafma0 rafma0 requested a review from a team as a code owner November 2, 2023 01:32
@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Nov 2, 2023

In webOS 1.2 without this hack (enableRefreshHack = false) emby-checkbox cannot be focused or even clicked.
So I guess the hack was introduced for old web engines such as WebKit.

Maybe related livereload/livereload-extensions#26 (comment)

Replacing padding with margin seems to work in webOS 1.2 and doesn't cause jitters.
More testing is welcome.

@dmitrylyzo dmitrylyzo added bug Something isn't working ui & ux This PR or issue mainly concerns UI & UX p: webos This PR or issue mainly concerns WebOS clients p: tizen This PR or issue mainly concerns Tizen clients labels Nov 2, 2023
@rafma0
Copy link
Contributor Author

rafma0 commented Nov 2, 2023

Hey @dmitrylyzo, examining the reference you provided, they direct to a Stack Overflow post where one of the most upvoted solutions is just adding transform: translateZ(0) to the element, and it should solve the problem similarly. I've pushed a change that includes this tweak. Could you please test it on webOS 1.2? If it doesn't work, I'll revert to the original code with the new margin change and test on my Tizen again.

@dmitrylyzo
Copy link
Contributor

dmitrylyzo commented Nov 2, 2023

Hey @dmitrylyzo, examining the reference you provided, they direct to a Stack Overflow post where one of the most upvoted solutions is just adding transform: translateZ(0) to the element, and it should solve the problem similarly. I've pushed a change that includes this tweak. Could you please test it on webOS 1.2? If it doesn't work, I'll revert to the original code with the new margin change and test on my Tizen again.

No, it doesn't work. That solution probably only works for a specific layout.

The web engine doesn't seem to redraw the checkboxOutline. The hack forces redrawing the entire label.

Also, I figured out why it shakes: here padding-left is not zero, but the animation sets it to zero.
So we probably just need to animate any property without changing it.

Or we need to completely rewrite emby-checkbox.

@rafma0
Copy link
Contributor Author

rafma0 commented Nov 2, 2023

The web engine doesn't seem to redraw the checkboxOutline. The hack forces redrawing the entire label.

What about using transform: translateZ(0); in checkboxOutline, do you think it's worth a try?

Okay, never mind. I've reset the branch and changed padding to margin. I tested it on Tizen, and it all looks good. This is probably a good fix for now.

Copy link

sonarcloud bot commented Nov 2, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@rafma0 rafma0 changed the title remove unneeded(?) refresh hack from emby-checkbox fix jittering in emby-checkbox Nov 2, 2023
@jellyfin-bot
Copy link
Collaborator

Cloudflare Pages deployment

Latest commit 0bedd5a
Status ❌ Failure. Check workflow logs for details
Preview URL Not available
Type 🔀 Preview

View build logs
View bot logs

@thornbill thornbill merged commit 5bbce92 into jellyfin:master Nov 3, 2023
23 checks passed
@rafma0 rafma0 changed the title fix jittering in emby-checkbox Fix jittering in emby-checkbox Nov 3, 2023
@rafma0 rafma0 deleted the fix-checkbox branch November 3, 2023 13:57
@dmitrylyzo
Copy link
Contributor

Need to port this to 10.8. There will be 2 more lines to change (legacy fallback block).

nyanmisaka added a commit to nyanmisaka/jellyfin-web that referenced this pull request Nov 29, 2023
Signed-off-by: nyanmisaka <nst799610810@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working p: tizen This PR or issue mainly concerns Tizen clients p: webos This PR or issue mainly concerns WebOS clients ui & ux This PR or issue mainly concerns UI & UX
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants