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

[touch/filemanager] - workaround for the non-working touch input in t… #8368

Merged
merged 1 commit into from
Dec 1, 2015

Conversation

Memphiz
Copy link
Member

@Memphiz Memphiz commented Nov 8, 2015

…he filemanager window.

This is a workaround for the non-working touch controls in the FileManager window. Basically you can't enter any directories in there.

As said - this is only a workaround and also only works partially. While the FileManager window is now usable with touch input again - in confluence for example the home and back buttons at the lower screen are not working. The real cause has to do with wrong evaluation order of messages/actions or with badly tracked focus state.

On touch we basically send a mouse move, then fire the action to be done on the selected item which has focus (focused by the mouse move) and then send an unfocus_all message. Strangly this doesn't work in the FIleManager window. There we see the mouse move, then the unfocus_all and then the SELECT_ITEM message (the latter then checks for the focused control and gets none and so it does nothing). Either that OR the order is correct but during the SELECT_ITEM handler there is no focused control reported.

I have informed @mkortstiege and @Paxxi about it a while back and also provided a branch for reproduction with mouse input here:

https://github.com/Memphiz/xbmc/commits/reproduce_touch_issue

No clue who else is into that messaging/gui focus stuff. For jarvis we need a working filemanager with touch input - either the real cause can be tracked down and fixed or this workround needs to go in.

Just for dissociation. I am not the developer who will track down the real cause (i missed to much in the last couple weeks and others are much quicker with this).

@Memphiz Memphiz added Type: Fix non-breaking change which fixes an issue v16 Jarvis labels Nov 8, 2015
@mkortstiege
Copy link
Member

Wouldn't https://gist.github.com/mkortstiege/7048189025f14a2b0dc2 be enough here? It's basically the same you did for the calibration window in a88928b

Unfortunately i have no idea about the touch stuff at all so i don't know why it's playing the (silly?) focus games.

@Memphiz
Copy link
Member Author

Memphiz commented Nov 9, 2015

The calibration window is different (there are no real controls there to focus. The silly focus game is just there to give feedback to the user that he has pressed something (same as most touch UIs give feedback) by focussing the control as long as the finger is tapped down on the display.

That focus_meh.diff - not sure what it does. Does it work with my "reproduce_touch_issue" branch? (which allows to reproduce the issue with a mouse)

@mkortstiege
Copy link
Member

Jep that's working but yet another hack i think. Will check again and hopefully come up with a better solution for the mouse/gesture handling.

@mkortstiege
Copy link
Member

From what i can see this is caused by a previous workaround for the doubleclick behavior in the filemanager window and dialog(s). I think it's okay to merge as as.

For the sake of consistency, i suggest we change the doubleclick stuff to single click / tap and use longpress for hightlighting in v17. IMO this window should not have special treatment when it comes to files and folder handling.

@Memphiz
Copy link
Member Author

Memphiz commented Nov 10, 2015

it has special treatment because you can highlight more then one item in the list (which makes it different to all other windows). longpress opens the context menu (like with other windows) which is already consistent for touch input. I will check your workaround to see if it fixes the home and back buttons too (which mine doesn't) - thx for digging into it matthias :)

@Paxxi
Copy link
Member

Paxxi commented Nov 10, 2015

I still have this on the todo list, can't make any promises for b1 but should soon have some time to look into a proper fix.
No objections to merging this in the meantime if it's important for ios

@mkortstiege
Copy link
Member

It feels wrong to workaround a workaround with a workaround. Nasty window ;)

@Memphiz
Copy link
Member Author

Memphiz commented Nov 10, 2015

not important to have it working now - it effects droid in the same way - it just needs to work when we release at the latest ;)

@Memphiz
Copy link
Member Author

Memphiz commented Nov 10, 2015

Holy - i just figured out that the whole unfocus_all stuff doesn't work anymore. Maybe to clear up the "importance" of this on touch devices:

This is isengard (all working):

https://dl.dropboxusercontent.com/u/30371861/Kodi_touch/isengard_touch_works.mov

And this is current mainline (unfocus not working):

https://dl.dropboxusercontent.com/u/30371861/Kodi_touch/jarvis_touch_no_unfocus.mov

It just looks and feels ugly.

Beside that your workaround is better @mkortstiege because it also makes the home and back buttons in confluence working when being in windowfilemanager.

@Paxxi @mkortstiege so any idea what broke the unfocus_all stuff?

@Paxxi
Copy link
Member

Paxxi commented Nov 30, 2015

Haven't had a chance to look at this yet and I don't seem to get much time to spend on it in the foreseeable future. So unless someone want to investigate I vote for merging this to get v16 out the door.

@Memphiz
Copy link
Member Author

Memphiz commented Nov 30, 2015

will change it to the proposal from mkortstiege (as its cleaner if it even can be called that haha) and then merge ... thx for getting back to me :)

@Memphiz
Copy link
Member Author

Memphiz commented Dec 1, 2015

jenkins build and merge

@jenkins4kodi jenkins4kodi merged commit 3685515 into xbmc:master Dec 1, 2015
@MartijnKaijser MartijnKaijser modified the milestone: Jarvis 16.0-beta3 Dec 1, 2015
@Memphiz Memphiz deleted the touch_workaround_filemanager branch December 1, 2015 15:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Fix non-breaking change which fixes an issue v16 Jarvis
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants