[guilib] fix modality handling of dialog slider (fixes #16140) #7571
Conversation
|
Give me a while Its compiling this now. |
|
sure no rush i never used that feature 😃 |
|
@xhaggi Pulled this PR ontop of master and the issue is still there. I dindt pull https://gist.github.com/mkortstiege/026fa815e2d8f9461967 in addition to yours which fixed the repeat keypress was I suppose to compile both? |
|
no only this PR please, i'll take another look at it. |
xhaggi
force-pushed
the
fix-modality-of-dialog-slider
branch
from
e17181a
to
732430d
Compare
|
@UniversaI i've updated the PR, please do another test, thanks. |
|
I'm not really happy with this fix, because IMO we should introduce a new dialog for the case we need it as an overlay. This way we have a cleaner implementation and a skinner could style the overlay different from the modal (the modal should hold an ok and close button). @mkortstiege @ronie what's your opinion? |
|
@xhaggi Confirmed fixes issue. 💃 Congrats and thank you :) |
|
Why not use my gist in case it fixes the issue? The slider should be handled just as the other osd/overlay Dialogs. |
|
skinners prefer less dialogs, not more ;-) why not make this dialog modal all of the times? |
Your gist fixes the issue but when you go into OSD manually and select any slider then it stays on screen forever, so it fixes the issue and breaks other use cases, the way @xhaggi did it works in any case. |
|
@ronie imo it's a design flaw to unify a modeless and modal dialog. if you want to add a button to close this dialog this won't work in the modeless mode. making this dialog only modal will break the case @UniversaI described. same happens if we make it modeless as suggested by @mkortstiege. we can't take care of what skinners want (less dialogs) if this breaks our internal design. sure this worked for years but only with some limitations like i previously said (no buttons to interact). |
|
jenkins build this please |
[guilib] fix modality handling of dialog slider (fixes #16140)
As reported at http://trac.kodi.tv/ticket/16140 the slider dialog does not work if it's used as overlay. This happens because we only support one modality type per dialog and the slider dialog is the only one which breaks this rule and is used as modal and overlay at the same time.
This PR will fix the issue by setting the modality type at runtime depending on the use-case.
@UniversaI it would be nice if you could confirm that it works
@mkortstiege mind taking a look