[Approved] Modified focus handling (1 Viewer)

Brownard

Development Group
  • Team MediaPortal
  • March 21, 2007
    2,290
    1,872
    Home Country
    United Kingdom United Kingdom
    Hi

    I've always felt like there was something not quite right with the focus handling in MP2, particularly how it chooses the purely closest element when there is another element that is more in line with the movement direction although further away. I've modified the handling slightly so that it takes this into account and it feels more intuitive to me (e.g pressing left on an item list takes you straight to the side menu rather than the navigation tree)

    There is one issue with my implementation (although I'd prefer to call it a bug with MP2 :)), when the side menu in Titanium is displayed the ActualBounds property of the menu elements doesn't take into account the menu's new location so it's difficult to re-enter focus on the menu after you've left it. Is this by design?

    What do you think?
     

    Attachments

    • SkinEngine.zip
      350.2 KB
    • 0001-Modify-focus-handling.patch
      6 KB

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    Thanks for this patch! I will check the code later.

    Did you already test all focus movement test screens inside GUI-Test plugin? They have some special cases for transformations of controls.
     

    Brownard

    Development Group
  • Team MediaPortal
  • March 21, 2007
    2,290
    1,872
    Home Country
    United Kingdom United Kingdom
    • Thread starter
    • Moderator
    • #3
    Did you already test all focus movement test screens inside GUI-Test plugin? They have some special cases for transformations of controls.

    I hadn't, but I just did and looks like my patch could do with a bit of tweaking ;). I think it's definitely an improvement when using 'Focus movement test 1' in that pressing up/down on the middle column keeps focus within the column rather than jumping to the left.

    It falls down a bit on test 2 as it skips rows when moving up/down because I'm calculating alignment based on the centre of the controls - I will see if I can improve this.
     

    Brownard

    Development Group
  • Team MediaPortal
  • March 21, 2007
    2,290
    1,872
    Home Country
    United Kingdom United Kingdom
    • Thread starter
    • Moderator
    • #4
    I've updated the first post with a new patch and bin which now works well with the GUI Tests.

    Incidentally Focus movement test 1 highlights the issue I was mentioning earlier with translated controls, shouldn't the focus prediction be using the translated position rather than the original position?
     

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    shouldn't the focus prediction be using the translated position rather than the original position?
    IIRC we (Albert and me) tested to include RenderTransform into consideration. But this leads to some strange side effects. Also WPF doesn't do this, so we decide to roll changes back. But LayoutTransform is considered though.
     

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    I just have applied your patch and I think the results are good (y)!

    I will keep the changes in my local test builds and test them in deep. Thanks a lot! Let me know if there are more things to improve :)
     

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    I tested your changes a while. Unfortunately I found some situations where your version does not work well. See following screens (default skin) as example. Focus is inside content area and I hit "down" once. I expect to stay in list, but focus jumps to main menu.
    01_Expected.png
    02_Actual.png

    Another example is the SlimTv home screen: if you are on channel list and move "Up", focus moves to left side.

    Can you check those examples? At least the first should be good reproducable. For a public testbuild I'd revert the changes so long, I hope you understand this.
     

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    Sorry, I had another change in code which introduces this behaviour. It was not your change.

    At least it shows how sensible this part is. I'm now looking for another solution of my problem.
     

    morpheus_xx

    Retired Team Member
  • Team MediaPortal
  • March 24, 2007
    12,073
    7,459
    Home Country
    Germany Germany
    Maybe you can help with my issue?
    I have tried to draw the layout: the main ScrollViewer (1) contains 3 nested ScrollViewers.
    ScrollViewerFocus.png

    Now I focus an item of (3) and hit "Down".
    Screen (top element) gets "Down" key and tries every child control to handle this key.
    (1) skips, because of "Check focus in scope" (https://github.com/MediaPortal/Medi...ne/Controls/Visuals/ScrollViewer.cs#L507-L512)

    (2) is skipped (CheckFocusInScope false)
    (3) has the focus in scope, but can't handle "Down" (end of list)

    Now we have a problem: the parent ScrollViewer (1) is already done with handling this key, so it doesn't scroll down.

    In my test I removed the CheckFocusInScope which caused trouble as I described above.

    Any ideas how to handle this? Bubble key up again to let parent try to handle it?
     

    Brownard

    Development Group
  • Team MediaPortal
  • March 21, 2007
    2,290
    1,872
    Home Country
    United Kingdom United Kingdom
    • Thread starter
    • Moderator
    • #10
    What about something like this?

    Basically each ScrollViewer calls KeyPressed on it's first child ScrollViewer, only if the child can't handle does the parent then handle it. This should cause the event to bubble up in the case of nested ScrollViewers.
     

    Attachments

    • 0001-Check-if-any-child-ScrollViewers-can-handle-key-pres.patch
      2.1 KB

    Users who are viewing this thread

    Top Bottom