xfangfang / borealis

Hardware accelerated, Nintendo Switch inspired UI library for PC, Android, iOS, PSV, PS4 and Nintendo Switch
Apache License 2.0
31 stars 21 forks source link

slider : add hidePointer() method #72

Open PoloNX opened 8 months ago

PoloNX commented 8 months ago

Write the function proposed here. Works fine

XITRIX commented 8 months ago

@xfangfang, sorry, if that was your proposal, I would not agree with it at all. This should be 2 completely different UI components, at least @PoloNX can inherit Slider to not duplicate code for now (and later rewrite it as separate component), but in current solutions having irrevertible public function is completely bad practice

I'd better say that previous PR was good enough, only namings and a bit of code clean up had to be done there. It was much better than this solution

xfangfang commented 8 months ago

@XITRIX It’s okay for me to accept the original pr, but I feel that there is a lot of reusable code between the new and current components, so there is no need to reimplement the basic parts. For me, perhaps inheriting the ProgressView from the previous PR and reimplementing the Slider would be a better idea, because as I mentioned before, the current slider component will experience jitter when progress changes occur.

XITRIX commented 8 months ago

Ok, I checked how HOS uses such UI element, and I kinda changed my mind IMG_6463 IMG_6464 It could be just a progress and draggable slider at the same time, so maybe this PR approach is a nice idea, btw it should include showPointer() as well, so this operation could be reverted back after hiding it

Speaking about jittering I also noticed it, not sure what is the source of it, probably floating point in width calculations causes it ... I could check if you want (I've made this slider several years ago btw :D)

PoloNX commented 8 months ago

Hi, just added the showPointer() method. Can someone review the code ?

XITRIX commented 8 months ago

Check, pls, that showPointer() is exact mirror of hidePointer(), i.e. I cannot see reverse for pointer->setTranslationX(-3.5); in showPointer() function

Also, I'm not sure that this functions should call setFocusable, hide\show Pointer is more like "styling" functions and I as developer do not expect that it will change behaviour as well, I think it's better to explicitly call setFocusable(false) on place, if you really need to disable it's interaction behaviours. I.e. like I've mentioned above, in HOS it hides pointer, but do not disable interactions with it

PoloNX commented 8 months ago

When you focus the pointer on Hos, is there a small blue selection circle or nothing at all? Perhaps it should be hidden if it's not there on Hos.

XITRIX commented 8 months ago

It appears when you start to drag it, you can check it's behaviour on your own in Album app

PoloNX commented 8 months ago

is it good ?