search

lbryio / lbry-desktop

A browser and wallet for LBRY, the decentralized, user-controlled content marketplace.
https://lbry.tech
MIT License
3.56k stars 414 forks source link

ui/components/subscribeButton has glitchy visual behavior. #2677

Closed abueide closed 4 years ago

abueide commented 5 years ago

The Issue

When hovering over the heart icon of the unsubscribeButton, glitchy visual behavior happens.

Steps to Reproduce

Hover over heart icon in a very specific pixel.

Recorded Video of the Bug

https://beta.lbry.tv/bug/f

tzarebczan commented 5 years ago

Thanks for opening the issue @abueide ! I was able to reproduce...,yeah, looks like it's in very particular spot.

Can we show you some appreciation for the issue?

kauffj commented 5 years ago

I love the usage of LBRY to report bugs on LBRY. It's LBRY all the way down!

@seanyesmunt Even with a resolution of this specific issue, I've found that it can create a feeling of "jumpiness" in other places. I really like the new behavior - maybe we need to fix the width?

abueide commented 5 years ago

Haha, yeah no matter how I tried to make the gif, it was bigger than github/imgur's limit of 10/20 mb, so I thought why sacrifice quality might as well upload to lbry.

ICTman1076 commented 5 years ago

I think I might have a crack at this. My initial thought is to change the character spacing ever so slightly so that "unfollow" is slightly wider, and "following" is either the same width or slightly thinner, so that the two become equal width and result in this glitchy situation to be resolved. Having different character spacing may make it look worse, but it's worth a shot, right?

ICTman1076 commented 5 years ago

^ Sorry for the first PR, I made a mistake on that one. #2990 is the one that should be merged.

VeroniqueHuang commented 4 years ago

Hi, I'd like to work on this if it's not fixed yet !

neb-b commented 4 years ago

@VeroniqueHuang It's all yours!

sloganking commented 4 years ago

Hello, I am looking at fixing this issue. This is my first time developing with LBRY and I am having trouble seeing my changes take place.

I am currently trying to modify the string 'Following' here

however after running yarn dev to compile the project I am not seeing that the text has changed. Any help is appreciated. I am currently using

https://spee.ch/7da73fc508ffc4ff8b2711e3c3950110430b0c5f/LBRYAppDesign.mp4

as a reference to start developing. If there is a more up to date reference I would be happy to look at it.

neb-b commented 4 years ago

Hey @SleepingFox88

That video is still (sort of) up to date. You might need to refresh when developing with Electron. If that still doesn't work maybe try yarn dev:web to develop against lbry.tv?

Let me know if that works.

sloganking commented 4 years ago

@seanyesmunt

The program successfully compiles with yarn dev:web and opens a browser tab to http://localhost:9090/ however the page title is "Error" and the only text on the web page is Cannot GET /

Chromium's development console displays this error

Firefox's development console displays something similar.

Edit:

I am attempting all this on Manjaro Linux if that matters.

neb-b commented 4 years ago

@SleepingFox88 Ah sorry about that. I forgot you need to run yarn compile:web first, since it's your first time building the app https://github.com/lbryio/lbry-desktop#run-the-web-app

sloganking commented 4 years ago

@seanyesmunt Thanks. The application compiled and launched after that but I am still not seeing the changes I made. Both the web app and the electron app are giving me three console errors after launching the application.

The error of "react does not recognise" occurs for isSelected (as seen in pic) _userInteractedRef and _onSelect

I am not sure if these are related to my problem or if they are naturally in the main branch.

EDIT: However I now notice the same issues are in this issue's reporter's screen capture so I am assuming they are unrelated

A quick test shows any console.log()'s I add aren't showing up in console either.

Edit:

I am going to try this on Windows 10 to see if my issue is system specific.

sloganking commented 4 years ago

Having the same issue on windows. Can anyone else verify that if they modify the Following string in lbry-desktop\ui\page\discover\view.jsx

that their compiled web or electron app updates the Following string in the top right?

sloganking commented 4 years ago

After further searching through the repository I see that string is updated in ui/component/subscribeButton/view.jsx instead of lbry-desktop\ui\page\discover\view.jsx. My apologies that was an error on my part. I can now confirm that my development app is successfully updating. Thank you @seanyesmunt for helping me set up.

kauffj commented 4 years ago

@seanyesmunt are there any README changes to make so that the next person has a better experience than @SleepingFox88? Thanks for getting set up Logan!

neb-b commented 4 years ago

Glad you got it working! Sorry I missed the wrong file name.

@kauffj Maybe, I'm not sure. Maybe we could point a string on the homepage that users can change to confirm it's updating?

sloganking commented 4 years ago

@kauffj My primary problem was that I was modifying the wrong file which was my own fault. I was searching this repository for a certain string and only saw the results on the first page of results instead of realizing that there were multiple pages of results.

In terms of what documentation could be improved, I first found this video in contributing.md which was marketed as a quickstart guide to running and developing the desktop app. I did run into one of my issues because that video just runs yarn dev to start the application without mentioning that the first time you run the application you need to run yarn compile:electron before you run yarn dev These instructions are found in this repo's readme but not in the video quickstart guide.

I should mention that he quickstart guide video also seems to be old enough that this repo's file structure has changed but I don't think that really stops people from getting started.

On another note, I saw it mentioned in a closed PR for this issue that LBRY supports multiple languages and our solution to this issue may need to work for all of them. If we have multi-language support, is there a way to test that our solution works with other languages as well? I don't suppose modifying the hard coded English strings would be accepted if this problem persists with other languages.

sloganking commented 4 years ago

I see from PR #2790 we have a way to manually set the pixel width of the subscribe button. Does anyone know a way to get the current pixel width and store it in a variable instead? I am not yet the most experienced with web development. Any tips or documentation is appreciated.

sloganking commented 4 years ago

https://github.com/sloganking/lbry-desktop/tree/a5800970960c292fcd5a9d3d4df7cd5b1c9cb8fd

So I have created a PoC for how I'd like to fix the issue. It's still messy in that my current implementation has an issue that causes the buttons to be larger than they should be and moves the content closer to the icon but that should be able to be fixed. I enabled borders around the button divs to aid in debugging but those would be removed before merging back to master.

I initially thought this issue was caused by the size of the subscriptionButton's hitbox shrinking when the mouse went over it, and expanding when the mouse was not over it. This would cause there to be a space on the screen where the mouse, without moving, would continually enter and exit the button's hitbox thus updating the visual state.

However in my testing I have found that the state jittering still occurs on occasion even after that button hitbox stays the same size. It happens less often when I enable the visual borders around the divs although I am not sure why.

https://streamable.com/06sfa2

This jittering only occasionally occurs when the mouse enters the heart icon from the left and stays hovered over it. If the user first mouses over the button label to the right of the heart icon, and then hovers over the heart icon, there does not appear to be any chance that jittering will occur until the mouse leaves the button hitbox. This is more than a 1 pixel issue as while jittering, the mouse is able to move over different parts of the heart icon and the issue still persists. because of this I am no longer certain what's causing the issue.

Edit:

Also as a separate issue, it should be noted that the subscribe button text can become in the opposite state that it should be if the state is changed very quickly. This can be seen in the last pat of the video above

neb-b commented 4 years ago

Hey @SleepingFox88

I'll check this out today!

ghost commented 4 years ago

Look for this: ICONS.UNSUBSCRIBE and issue #2272

Maybe the svg path should be examined here :

https://github.com/lbryio/lbry-desktop/pull/2272/commits/85fbea818008b5f61f78d775fb0076f76cb80117#diff-c4ac894f79900ba50f77cf36fe95fd39

https://github.com/lbryio/lbry-desktop/pull/2272/commits/85fbea818008b5f61f78d775fb0076f76cb80117#

The icon changes shape by adding the central part, the rollover is done and undone immediately.

lbry-desktop/ui/component/common/icon-custom.jsx

It would be nice to see if with another icon for the unsubscribe state this also happens.

neb-b commented 4 years ago

@SleepingFox88 I left a comment on your latest commit. I think something like that should work. We would just need to make sure the hidden label isn't read by screen readers.

Can you open a PR? (FYI there are some recent changes to those buttons)