borealis icon indicating copy to clipboard operation
borealis copied to clipboard

slider : add hidePointer() method

Open PoloNX opened this issue 1 year ago • 8 comments

Write the function proposed here. Works fine

PoloNX avatar Feb 27 '24 11:02 PoloNX

@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

XITRIX avatar Feb 27 '24 23:02 XITRIX

@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.

xfangfang avatar Feb 28 '24 15:02 xfangfang

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)

XITRIX avatar Feb 28 '24 18:02 XITRIX

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

PoloNX avatar Feb 29 '24 18:02 PoloNX

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

XITRIX avatar Mar 01 '24 08:03 XITRIX

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.

PoloNX avatar Mar 01 '24 09:03 PoloNX

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

XITRIX avatar Mar 01 '24 09:03 XITRIX

is it good ?

PoloNX avatar Mar 03 '24 15:03 PoloNX