bbc/lrud

Left, Right, Up, Down. A spatial navigation library for devices with input via directional controls.

LRUD

A spatial navigation library for devices with input via directional controls

🔩 Maintenance Mode 🔩

LRUD is now in maintenance mode; existing bugs will be fixed, but new features will not be added. A new library called LRUD Spatial is now available to the Open Source community.

Upgrading from V2

v3 is a major rewrite, covering many new features. However, it unfortunately breaks some backwards compatibility.

We are currently in the process of writing more detailed docs for an upgrade process. However, the main things to note at the minute at;

• changes in events, which ones are emitted and what they are emitted with
• removal of grid in favour of isIndexAligned behaviour

Getting Started

git clone [email protected]:bbc/lrud.git lrud
cd lrud
npm install

Lrud is written in Typescript and makes use of mitt.

Usage

npm install lrud

const { Lrud } = require('Lrud')

// create an instance, register some nodes and assign a default focus
var navigation = new Lrud()
navigation
  .registerNode('root', { orientation: 'vertical' })
  .registerNode('item-a', { parent: 'root', isFocusable: true })
  .registerNode('item-b', { parent: 'root', isFocusable: true })
  .assignFocus('item-a')

// handle a key event
document.addEventListener('keypress', (event) => {
  navigation.handleKeyEvent(event)
});

// Lrud will output an event when it handles a move
navigation.on('move', (moveEvent) => {
  myApp.doSomethingOnNodeFocus(moveEvent.enter)
})

See usage docs for details full API details.

For more "full" examples, covering common use cases, check the recipes

Running the tests

All code is written in Typescript, so we make use of a tsconfig.json and jest.config.js to ensure tests run correctly.

Test files are split up fairly arbitrarily, aiming to have larger sets of tests broken into their own file.

npm test

To run a specific test file, use npx jest from the project root.

npx jest src/lrud.test.js

You can also run all the tests with verbose output. This is useful for listing out test scenarios to ensure that behaviour is covered.

npm run test:verbose

You can also run all the tests with coverage output

npm run test:coverage

Several of the tests have associated diagrams, in order to better explain what is being tested. These can be found in ./docs/test-diagrams.

We also have a specific test file (src/build.test.js) in order to ensure that we haven't broken the Typescript/rollup.js build.

Versioning

npm version <patch:minor:major>
npm publish

Built with

• Typescript
• rollup.js
• mitt

Inspiration

• BBC - TV Application Layer (TAL)
• Netflix - Pass the Remote
• Mozilla - Implementing TV remote control navigation

Alternatives

• tal
• react-tv-navigation
• react-key-navigation
• js-spatial-navigation

License

LRUD is part of the BBC TAL libraries, and available to everyone under the terms of the Apache 2 open source licence (Apache-2.0). Take a look at the LICENSE file in the code.

Copyright (c) 2018 BBC

lrud's Issues

Having an issue in my app related to LRUD clearing `currentFocusNode`

I'm unsure why this is causing an issue all of a sudden for my newest refactor but the only thing that fixed my app was to comment out these 3 lines.

Been bashing my head against my desk all day trying to figure out what was causing my app to lose focus. I'm sure it's something I could avoid by doing things differently but figured I'd still bring it to your attention in case that isn't getting into code coverage or something.

lrud/src/index.ts

Line 42 in a6faaae

this.currentFocusNode = null

Let me know if there is anything I can test for you on my end.

on('select') listener does not fire unless node has its own onSelect method

Describe the bug
If I just want the same select function to fire, I would just set up a nav.on('select') listener. But this does not seem to work unless a node also has its own onSelect method.

To Reproduce

const nav = new Lrud()

nav.on('select', node => console.log(node.selectAction.route))

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.register('item1', {
  parent: 'root',
  selectAction: {
    route: '/home'
  }
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'enter' })

Expected behavior
I would expect to see my route logged in the console. But I don't... unless I add an onSelect method like this:

const nav = new Lrud()

nav.on('select', node => console.log(node.selectAction.route))

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.register('item1', {
  parent: 'root',
  selectAction: {
    route: '/home'
  },
  onSelect: () => {}
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'enter' })

Previously unregistered nodes reappear

Describe the bug
When a node has been unregistered, all its children are also unregistered. This appears to be the case, however when a node is added with an identical name, the old node can come back under some circumstances.

To Reproduce
Steps to reproduce the behavior:

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'horizontal'
})
nav.registerNode('node1', {
  orientation: 'vertical',
  parent: 'root'
})
nav.registerNode('container', {
  orientation: 'vertical',
  parent: 'node1'
})
nav.registerNode('item', {
  selectAction: {},
  parent: 'container'
})

nav.unregisterNode('root')

nav.registerNode('root', {
  orientation: 'horizontal'
})
nav.registerNode('node2', {
  orientation: 'vertical',
  parent: 'root'
})
nav.registerNode('container', {
  orientation: 'vertical',
  parent: 'node2'
})
nav.registerNode('item', {
  selectAction: {},
  parent: 'container'
})

console.log(nav)

Expected behavior
It should give a tree of structure:

root
-node2
--container
---item

It actually gives a tree of structure:

root
-node1
--container
---item
-node2
--container

So it seems node1 has returned and item is in thecontainer under node1 when it should be in thecontainer under node2

Error when unregistering node

Describe the bug
When a node is unregistered, LRUD tries to assign focus to something else. This seems to cause an issue when the node being unregistered is higher in the tree than the currently focused node.

To Reproduce

const nav = new Lrud()

nav.register('root', {
  orientation: 'horizontal'
})

nav.register('node1', {
  orientation: 'vertical',
  parent: 'root'
})

nav.register('node2', {
  orientation: 'vertical',
  parent: 'root'
})

nav.register('item1', {
  parent: 'node1',
  selectAction: {}
})

nav.register('item2', {
  parent: 'node2',
  selectAction: {}
})

nav.assignFocus('node2')

nav.unregisterNode('item1')

Expected behavior
Should remove item1. Instead, LRUD throws an error.

index.min.js:158 Uncaught TypeError: Cannot read property 'id' of null
    at e.digDown (index.min.js:158)
    at e.unregisterNode (index.min.js:86)

Personally, I would prefer LRUD not to attempt to focus on anything when I remove a node.

currentFocusNodeIndexRange questions

I'm a bit confused as to how to get isWrapping working with my current setup....

Am I supposed to be focusing the "carousel" or the "slide" within the carousel?
Everytime I focus my horizontal node it sets currentFocusNode to one of the slides.
Then its setting these values... (for example)

currentFocusNodeId: "content-recent-449" 
currentFocusNodeIndex: 9 
currentFocusNodeIndexRange: null 
currentFocusNodeIndexRangeLowerBound: 9 
currentFocusNodeIndexRangeUpperBound: 9

The thing I'm trying to do is on the parent of this node, move the position so it appears to be carouseling as the user navigates with the keyboard.

I had this fully working with 2.7.x and I'm working to transfer the logic to 3.5.x.

Do I need to make my carousel isFocusable: true?
The docs seem to imply that the parent to a list should not be focusable...?

Idea: Autofocus

For accessibility reasons (screenreader support) it is sometimes useful to have the browser's focus linked to the focus state of the LRUD tree.

At the moment to do this you can set the tabindex for every element in the LRUD tree to "-1" and then add a listener for the "focus" event that sets the DOM focus to match the element, we could maybe make this process a little easier by having a config option to automatically add the tabindexes and keep the browser focus state in sync with the LRUD focus.

Any thoughts?

Support cancellation of movement

It is a common use case that some business logic needs to be performed after a move is initiated by the user but prior to the completion of the movement. The business logic may impact whether the completion of the movement occurs at all. Examples of this are failure to retrieve data, updating of internal state, validation that the initiated action is acceptable.

In these scenarios, some mechanism to allow the application to halt movement after it has been initiated and allow it to be completed after a successful business action has occurred would be ideal.

Possible solutions to this problem are:

• Being able to cancel a move operation, via the return of a boolean value from a move callback, or providing an API object method such as .cancelEvent()
• The move callback invoking the application handler with a callback function which could be invoked after the business logic is complete
• The move callback supporting promise-returning handlers, allowing continuation after the business logic is complete

Unregistering a node if there are no other focusable nodes

If you unregister a node, and if there is nothing else that can be focused then there is an error:

const nav = new Lrud()
nav.registerNode('root', {orientation: 'vertical'})
nav.registerNode('row1', {orientation: 'horizontal', parent: 'root'})
nav.registerNode('item1', {selectAction: {}, parent: 'row1'})
nav.unregisterNode('row1') // Uncaught TypeError: Cannot read property 'activeChild' of null

This is fine however as there is another focusable node:

const nav = new Lrud()
nav.registerNode('root', {orientation: 'vertical'})
nav.registerNode('row1', {orientation: 'horizontal', parent: 'root'})
nav.registerNode('item1', {selectAction: {}, parent: 'row1'})
nav.registerNode('row2', {orientation: 'horizontal', parent: 'root'})
nav.registerNode('item2', {selectAction: {}, parent: 'row2'})
nav.unregisterNode('row1')

Overrides of children are not cleaned up when unregistering parent node

Describe the bug
When node is unregistered then its overrides are cleaned as well, but if such node contains children that have overrides defined as well, than such overrides (those belonging to children) are not cleaned up.

To Reproduce

  test('unregistering a node should unregister the overrides of its children', () => {
    const navigation = new Lrud()

    navigation.registerNode('root')
    navigation.registerNode('a', { parent: 'root' })
    navigation.registerNode('ab', { parent: 'a' })
    navigation.registerNode('b', { parent: 'root' })

    navigation.registerOverride('override_a', {
      id: 'ab',
      direction: 'right',
      target: 'b'
    })

    navigation.unregister('a')

    expect(navigation.overrides).toEqual({})
  })

results with

 FAIL  src/unregister.test.js
  ● unregisterNode() › unregistering a node should unregister the overrides of its children

    expect(received).toEqual(expected) // deep equality

    - Expected  - 1
    + Received  + 7

    - Object {}
    + Object {
    +   "override_a": Object {
    +     "direction": "right",
    +     "id": "ab",
    +     "target": "b",
    +   },
    + }

       |     navigation.unregister('a')
       |
    >  |     expect(navigation.overrides).toEqual({})
       |                                  ^
       |   })
       |

Expected behavior
Expected behavior is that all overrides related to unregistered node and its children should be cleaned up.

Screenshots
No screenshots.

Additional context
No additional context.

[Maintenance] Replace all the `any` with TS types

Is your feature request related to a problem? Please describe.

We have lots of "any" as type, we should fix this.

Describe the solution you'd like
A clear and concise description of what you want to happen.
Add the correct type for all the any

Empty nodes cause problems when trying to focus/navigate

Describe the bug
When a node contains no focusable items, LRUD throws an error when trying to find an appropriate focusable item.

Case 1 - initial focus

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item2', {
  parent: 'node2',
  selectAction: {}
})

nav.assignFocus('root')

Expected behavior
Should focus on item2 but instead throws error:
Uncaught TypeError: Cannot read property 'id' of undefined

Case 2 - moving to empty node

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item1', {
  parent: 'node1',
  selectAction: {}
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'down' })

Expected behavior
Should remain focused on item1 but instead throws error:
Uncaught TypeError: Cannot read property 'id' of undefined

Case 3 - moving past empty node to item beyond it

To Reproduce

const nav = new Lrud()

nav.registerNode('root', {
  orientation: 'vertical'
})

nav.registerNode('node1', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node2', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.registerNode('node3', {
  orientation: 'horizontal',
  parent: 'root'
})

nav.register('item1', {
  parent: 'node1',
  selectAction: {}
})

nav.register('item3', {
  parent: 'node3',
  selectAction: {}
})

nav.assignFocus('root')

nav.handleKeyEvent({ direction: 'down' })

Expected behavior
Should focus on item3 but instead throws error:
Uncaught TypeError: Cannot read property 'id' of undefined

Can not focus node that contains only non focusable nodes

Describe the bug
When node containing only non focusable nodes is about to be focused than error is thrown: Error: "a" does not have focusable children. Are you trying to assign focus to a?

To Reproduce

test('should focus node with only non focusable children', () => {
    const navigation = new Lrud()

    navigation.registerNode('root', { orientation: 'vertical' })
    navigation.registerNode('a', { parent: 'root', isFocusable: true })
    navigation.registerNode('aa', { parent: 'a' })

    navigation.assignFocus('a')

    expect(navigation.currentFocusNodeId).toEqual('a')
})

Expected behavior
To have a node focused.

Additional context
Changing aa child to be focusable allows to focus node a.

currentFocusNode is horizontal but left/right aren't firing :-/

The up/down events fire but left/right aren't firing for me.
Is there a limitation with how nested nodes can be?

UPDATE: I thought it had to do with the fact that parent was horizontal and isFocusable as well as the children, but when I turn off isFocusable for the parent it throws Uncaught TypeError: Cannot read property 'id' of undefined

Registering node with already occupied index makes the index out of sync

Describe the bug
It is possible to register node with index value given in registration options. Such index is preserved. If another node is registered under the parent that already has child at a given index, than both children share the same index value. In such case the moment of adding takes precedence while searching for next or prev node, which in fact results in an unexpected behavior.

To Reproduce

  test('coherent index, should insert node at a given position', () => {
    const navigation = new Lrud()
    navigation.registerNode('root')
    navigation.registerNode('a', { parent: 'root' })
    navigation.registerNode('b', { parent: 'root' })

    navigation.registerNode('c', { parent: 'root', index: 1 })

    expect(navigation.getNode('c').index).toEqual(1)
    expect(navigation.getNode('b').index).toEqual(2) // but it is '1'
  })

results with

 FAIL  src/register.test.js
  ● registerNode() › coherent index, should insert node at a given position

    expect(received).toEqual(expected) // deep equality

    Expected: 2
    Received: 1

       |
       |     expect(navigation.getNode('c').index).toEqual(1)
    >  |     expect(navigation.getNode('b').index).toEqual(2) // but it is '1'
       |                                           ^
       |   })
       | })
       |

Expected behavior
The newly inserted child should have defined index value preserved and the existing child at that position should be shifted up.

Screenshots
No screenshots

Additional context
It's quite common situation that sometimes nodes can be prepended or inserted in the middle. To achieve such functionality index value needs to be recalculated outside LRUD library, which shouldn't be done that way.

Focus can not jump over last node that is not focusable

Describe the bug
When nodes are grouped into separate parents, the focus can not jump from parent 'A' to parent 'B' if last node of the parent 'A' is not focusable.

To Reproduce

test('should jump to first focusable node in other branch', () => {
    const navigation = new Lrud()

    navigation.registerNode('root', { orientation: 'vertical' })

    navigation.registerNode('a', { parent: 'root', orientation: 'vertical' })
    navigation.registerNode('aa', { parent: 'a', isFocusable: true })
    navigation.registerNode('ab', { parent: 'a' })

    navigation.registerNode('b', { parent: 'root', orientation: 'vertical' })
    navigation.registerNode('ba', { parent: 'b', isFocusable: true })
    navigation.registerNode('bb', { parent: 'b', isFocusable: true })

    navigation.assignFocus('aa')

    navigation.handleKeyEvent({ direction: 'down' })

    expect(navigation.currentFocusNodeId).toEqual('ba')
})

Expected behavior
Expected to have ba node focused.

Additional context
We can think of the leafs as a buttons. Some buttons might be disabled on a page because of its actual sate. That means such nodes should be set as not focusable in a LRUD navigation tree. After that focus is stuck and can not traverse appropriately through other buttons.

When node ab is set as focusable, than focus can easily traverse down from aa through ab -> ba -> bb.

LRUD gets confused when encountering empty nodes

Describe the bug
Sometimes LRUD is unable to find the appropriate node to focus on. This seems to happen when it encounters a node which does not contain any focusable children, even if the adjacent node does.

To Reproduce

const navigation = new Lrud()

window.navigation = navigation

navigation.registerNode('root', { orientation: 'horizontal' })
navigation.registerNode('column-a', { parent: 'root', orientation: 'vertical' })
navigation.registerNode('column-a-node-1', { parent: 'column-a', isFocusable: true })
navigation.registerNode('column-b', { parent: 'root', orientation: 'vertical' })
navigation.registerNode('column-b-row-1', { parent: 'column-b', orientation: 'horizontal' }) // contains no focusable nodes
navigation.registerNode('column-b-row-2', { parent: 'column-b', orientation: 'horizontal' })
navigation.registerNode('column-b-row-2-node-1', { parent: 'column-b-row-2', isFocusable: true })

// Focus on column a
navigation.assignFocus('column-a-node-1')

// Try and move right to column b
navigation.handleKeyEvent({ direction: 'right' }) // Uncaught TypeError: Cannot read property 'id' of null

Expected behavior
I would expect column-b-row-2-node-1 to be focused.

Method insertTree incorrectly maintain index when replacing first child

Describe the bug
When inserting a tree which top node is an existing node, which is a first child, that such original node index is not maintained. Inserted tree is appended instead of being left at first position.

To Reproduce

  test('should correctly maintain index when replacing first child', () => {
    const navigation = new Lrud()

    navigation.registerNode('root')
    navigation.registerNode('a', { parent: 'root' })
    navigation.registerNode('b', { parent: 'root' })
    navigation.registerNode('c', { parent: 'root' })

    navigation.insertTree({ a: { parent: 'root', isFocusable: true } })

    // expect top node was replaced with inserted tree
    expect(navigation.getNode('a').isFocusable).toEqual(true)

    // expect index of the top node parent is maintained
    expect(navigation.getNode('a').index).toEqual(0) // but it is equal `2`
  })

Expected behavior
Expected behavior is to have index kept. But instead:

  ● insertTree() › should correctly maintain index when replacing first child

    expect(received).toEqual(expected) // deep equality

    Expected: 0
    Received: 2

       |
       |     // expect index of the top node parent is maintained
    >  |     expect(navigation.getNode('a').index).toEqual(0) // but it is equal `2`
       |                                           ^
       |   })
       | })
       |

Screenshots
No screenshots.

Additional context
No additional context.

Registering/Unregistering sibling nodes that start with the same string causes issues

Describe the bug
Internal LRUD state uses string paths to determine some of its logic.

The processes around these is broken if 2 siblings start with the same series of characters

To Reproduce

nav.registerNode('root');
nav.registerNode('brand');
nav.registerNode('brand-content');

nav.unregisterNode('brand');

After this unregister call, brand-content is still inside the tree, but a call to getNode('brand-content') will return undefined, as its been removed from internal state.

Expected behavior
An unregister should not break internal state around pathing.

Using setActiveChild on an element with a parent that isn't currently focused de-syncs activeChild and currentFocusNode

Describe the bug
When setActiveChild is called on an element that's a leaf with a parent that's not the activeChild of its own parent the activeChild of the parent of a focused element can change, leading to navigation problems.

To Reproduce
Test case

const navigation = new Lrud()
navigation
        .registerNode('root')
        .registerNode('a', { parent: 'root' })
        .registerNode('b', { parent: 'root' })
        .registerNode('a0', { isFocusable: true, parent: 'a' })
        .registerNode('b0', { isFocusable: true, parent: 'b' })

      navigation.assignFocus('b0')
      navigation.setActiveChild('a', 'a0')
      expect(navigation.currentFocusNodeId).toEqual('b0')
      expect(navigation.getNode('root').activeChild).toEqual('b')

First expect passes indicating focus hasn't changed, second expect fails, so the focused element is no longer in the path of active children.

Expected behavior
setActiveChild should probably not bubble upwards, or maybe should have an option to disable bubbling, the current behaviour can cause problems when changing the activeChild on a different section of the tree.

What would cause override to focus second child of its target instead of the first?

Version: v3.5.1

And then when I move back up, it focuses the first element of the target which is correct...

I've tried to see if maybe somehow I was sending events twice or duplicating something but it's not ever hitting the first of the target when moving down to it.

[UPDATE]

Also, when I manually call the lrud lib and focus the same target manually it focuses the first one...

Only happens when I'm pushing down.

Any body out there know of something similar to this for the Roku platform? (lrud specificly (not javascript))

Why is a node not considered "focusable" even if it has children that are and even though assignFocus works on said node?

exports.isNodeFocusable = function (node) { return !!(node.selectAction || node.isFocusable); };

I'm getting to the bottom of a climbUp and it's skipping one of my parent node's because it doesn't have isFocusable set to true... yet if you do assignFocus('playlists') it focuses the first item in it as it should.

Anybody have any suggestions?

failure when using setNodefocusable

Describe the bug
Under certain cases using setNodeFocusable can break navigation.
Tested in 5.0.9

To Reproduce

const tree = {
 root: {
   children: {
      orientation: 'vertical', 
      a: {
         orientation: 'horizontal',
         children: {
           aa:  { isFocusable: true },
           ab: { isFocusable: true },
           ac: { isFocusable: true }
         },
     b: { isFocusable: true }
}

const lrud = new Lrud()
lrud.registerTree(tree)
lrud.assignFocus('b')
// activeChild of a is 'aa'
lrud.setNodeFocusable(aa, false)
// activeChild of a is 'ab'
lrud.setNodeFocusable(ab, false)
// activeChild of a is 'ab'

lrud.handleKeyEvent({direction: 'up'}) 
// TypeError: "Cannot read property 'id' of null"

Expected behavior
It should set the correct activechild and then not explode when there's a valid navigable node.

Uncaught RangeError: Maximum call stack size exceeded

Describe the bug
If the navigation tree has node with id 'undefined' (string containing 9 chars word) then climbUp method fails with Maximum call stack size exceeded when no focus candidates found.

When the root node is reached than climbing should stop but apparently it jumps (climbs up) to node with id 'undefined', and continue climbing, until root node is reached again, and again, and again, until call stack size is exceeded.

To Reproduce

const navigation= new Lrud()

navigation.registerNode('root', { orientation: 'horizontal' })
navigation.registerNode('undefined', { parent: 'root', isFocusable: true })

navigation.assignFocus('undefined')

navigation.handleKeyEvent({ direction: 'right' });

Expected behavior
Handling key event should be stopped and currently focused node should stay focused.

Additional context
On more complicated navigation trees it may happen than, before exceeding call stack size, focus suddenly jumps to node with id 'undefined'. Similar issue is for node with id 'null'. Additional side effects of this bug:

• navigation.assignFocus() - assigns focus to 'undefined' node.
• navigation.assignFocus(null) - assigns focus to 'null' node.
• navigation.pickNode() - unregisters 'undefined' node.
• navigation.pickNode(null) - unregisters 'null' node.
• etc...

v5.0.6 not pushed to npm?

Just curious!

Question about forceRefocus

Why is it that we use "assignFocus" if something is unregistered instead of like... setting the activeChild but not moving focus to it?

If I want to focus on something, but have some things "unregister" in the background...

It's stealing focus away from my intended focus because I'm unregistering some things.

That being said, I'm aware that I can set forceRefocus to false... but what I'd really prefer is to simply setActiveChild.

I wonder if there is a way we could simply setActiveChild and optionally refocus if that parent is actually already focused... somehow?

reindexChildrenOfNode doesn't seem to work right > 9 items...

[
0: 0,
1: 1,
2: 10,
3: 11,
4: 12,
5: 13,
6: 14,
7: 15,
8: 16,
9: 17,
10: 18,
11: 19,
12: 2,
13: 20,
14: 21,
15: 22,
16: 23,
17: 24,
18: 3,
19: 4,
20: 5,
21: 6,
22: 7,
23: 8,
24: 9,
]

Which is why wrapping isn't working above 9 items... and why getNodeLastChild() is returning only (at most 9 for me).

Relates to #29

Let me know if I'm just being silly here... 💯

Should I use insertTree or registerTree?

In usage.md, there is a section named insertTree(), but all of the example code uses instance.registerTree(). Is this a typo? Which one should I use?

Default key codes and matching devices

I've been looking into the default set of key codes provided in lrud and I can't figure some of them out. My approach has been to search the tal repo device configs to find matching devices.

The following don't match any device:

218: left,
217: right,
215: up,
216: down,

Even googling them I couldn't find anything useful. What device/s are they intended for?

Similarly, the following key codes are only used on Universal Windows Platform (UWP):

214: left,
205: left,
213: right,
206: right,
211: up,
203: up,
212: down,
204: down,
195: enter,

Is it worth including key code definitions only used on a single platform by default? Or maybe do they also apply to some other platform/s? Of course, it does no harm to include them but I'm just curious about what the reasoning is behind including them 🙂

Add a meta field for nodes to freely store data

Is your feature request related to a problem? Please describe.

We're integrating LRUD in a react-native app, and we frequently need to access the data linked to our node. Most notably the ref of our element, and some other stuff.

Concerning the ref: the example on the README says that we simply could get our element in the DOM by id document.getElementById(id) to get its ref. But as we're using React Native, this is not possible!
Ideally, we'd love to store the ref in a meta field of the node.

Describe the solution you'd like

It would be lovely to simply be able to store meta data in the node.

lrud.registerNode(id, {
  meta: {
    anything: 'I want',
  }
})

Describe alternatives you've considered

We had no other idea 🤷

Additional context

None yet.

(also: THANK YOU for this amazing lib. You've done an amazing job, and this is a lifesaver ❤️ )

Can keyCode be made optional in KeyEvent interface?

Is your feature request related to a problem? Please describe.
I would like the KeyEvent type for the argument passed to handleKeyEvent to not make KeyCode mandatory. The tests only use direction so I'm assuming the code works without KeyCode.

Describe the solution you'd like
Optional KeyCode in types.

export interface KeyEvent {
    keyCode?: number;
    direction: string;
}

Describe alternatives you've considered
See my workaround below.

Additional context
I'm working in a module that has already received an event translated into a simple string like 'left', 'up', etc. so I'd like to be able to call

navigation.handleKeyEvent({ direction: 'up' });

instead of

navigation.handleKeyEvent({ direction: 'up', keyCode: NaN });

Active parents don't always deactivate when in a nested tree

Describe the bug
Consider the following tree:

VerticalRoot
|-- HorizontalList
|   |-- Button1
|-- VerticalList
|   |-- Row1
|   |   |-- Button2
|   |-- Row2
|   |   |-- Button3

1. Focus begins on Button1.
2. The user presses down and focus moves to Button2. HorizontalList becomes inactive. VerticalList and Row1 become active.
3. The user presses down and focus moves to Button3. Row1 becomes inactive. Row2 becomes active.
4. The user presses up and focus moves to Button2. Row2 becomes inactive. Row1 becomes inactive.
5. The user presses up and focus moves to Button1. VerticalList becomes inactive. HorizontalList becomes active. Row1 remains active, however.
6. The user presses down and focus moves to Button2. VerticalList becomes active and emits an onActive event, but Row1 does not, as it was already active.

The above sequence is an issue if you expect some behaviour to happen when Row1 becomes active. In my case, I want the page to scroll to each of the rows when they become active. This happens most of the time, but not in the case of step 6.

To Reproduce
Repeat the above steps using the following code:

const { Lrud } = require('Lrud');

const navigation = new Lrud();

const onFocus = (node) => console.log('focus:', node.id);
const onBlur = (node) => console.log('blur:', node.id);
const onActive = (node) => console.log('active:', node.id);
const onInactive = (node) => console.log('inactive:', node.id);

navigation
    .registerNode('VerticalRoot', { orientation: 'vertical' })
    .registerNode('HorizontalList', { orientation: 'horizontal', onActive, onInactive })
    .registerNode('Button1', { isFocusable: true, parent: 'HorizontalList', onFocus, onBlur })
    .registerNode('VerticalList', { orientation: 'vertical', onActive, onInactive })
    .registerNode('Row1', { orientation: 'horizontal', parent: 'VerticalList', onActive, onInactive })
    .registerNode('Button2', { isFocusable: true, parent: 'Row1', onFocus, onBlur })
    .registerNode('Row2', { orientation: 'horizontal', parent: 'VerticalList', onActive, onInactive })
    .registerNode('Button3', { isFocusable: true, parent: 'Row2', onFocus, onBlur });

navigation.assignFocus('Button1');

window.addEventListener('keydown', (event) => {
    console.log('key:', event.code);
    navigation.handleKeyEvent(event);
});

You should get the following log:

 1 active: HorizontalList
 2 focus: Button1
 3 key: ArrowDown
 4 blur: Button1
 5 active: Row1
 6 inactive: HorizontalList
 7 active: VerticalList
 8 focus: Button2
 9 key: ArrowDown
10 blur: Button2
11 inactive: Row1
12 active: Row2
13 focus: Button3
14 key: ArrowUp
15 blur: Button3
16 inactive: Row2
17 active: Row1
18 focus: Button2
19 key: ArrowUp
20 blur: Button2
21 inactive: VerticalList
22 active: HorizontalList
23 focus: Button1
24 key: ArrowDown
25 blur: Button1
26 inactive: HorizontalList
27 active: VerticalList
28 focus: Button2

Expected behaviour
There should be inactive: Row1 at log line 21. There should be active: Row1 at line 26.

Focused node becomes unfocused when separate part of tree removed

Describe the bug
When a node is focused, and another part of the tree is removed (using forceRefocus: false), the focused node becomes unfocused BUT the on blur method is not called.

To Reproduce
Steps to reproduce the behavior:

const navigation = new Lrud()

window.navigation = navigation

navigation.registerNode('root', { orientation: 'horizontal' })
navigation.registerNode('column-a', { parent: 'root', orientation: 'vertical' })
navigation.registerNode('column-a-node-1', { parent: 'column-a', isFocusable: true })
navigation.registerNode('column-b', { parent: 'root', orientation: 'vertical' })
navigation.registerNode('column-b-node-1', { parent: 'column-b', isFocusable: true })

navigation.on('focus', node => {
  node.isCustomFocused = true
})

navigation.on('blur', node => {
  node.isCustomFocused = false
})

console.log(navigation)

// Focus on column a
navigation.assignFocus('column-a-node-1')
console.log(navigation.getNode('column-a-node-1').isCustomFocused) // On focus has been called
console.log(navigation.currentFocusNode) // Node is focused

// Try and move right to column b
navigation.unregisterNode('column-b-node-1', { forceRefocus: false })
console.log(navigation.getNode('column-a-node-1').isCustomFocused) // On focus has not been called
console.log(navigation.currentFocusNode) // No node is not focused

Expected behavior
Either the focused node which is not removed should remain focused, or at least when the focused node is unfocused the on blur function should be called.

Asyncronous adding items at higher index than total list count

Is your feature request related to a problem? Please describe.
When building the UI, the application is asynchronously loading content separately for 3 components. Depending on the API response time, some are finished before others. They index ordering is 0,1,2 but they may get built in random order (register 2, register 0, register 1). I'm passing the index on the registration command, but if I add a node with index 2 on a list of 1 node, it will get added in index 1, thus conflicting with the subsequent node.

Describe the solution you'd like
The node should be added in the correct index even with a shorter list. Perhaps lrud could add a "ghost" or temporary node between the non existing node index, and replace it with the real node when it gets registered with that index

Describe alternatives you've considered
Having an abstraction that will receive the registration events but only passing them to lrud once all of them are loaded, but it seems like the library should be able to handle that by itself

LRUD not outputting ES5 or including polyfills.

Describe the bug

LRUD targets ES5 but uses ES6 features (Array.prototype.find, String.prototype.includes).
Typescript should be checking for usage of features at a newer version but isn't. Alternatively we should be including polyfills.

To Reproduce
tsc runs without error

Expected behavior
tsc should complain that we're using es6 features

Can set a node's activeChild to be a non-focusable element

Describe the bug
It is possible to set a non-focusable node to be the activeChild of a parentElement, this can cause problems changing focus when a different child of that parentElement is focused.

To Reproduce

      const navigation = new Lrud()
      navigation
        .registerNode('root')
        .registerNode('parent', { orientation: 'vertical' })
        .registerNode('a', { parent: 'parent' })
        .registerNode('b', { parent: 'parent', isFocusable: true })

      navigation.assignFocus('b')
      navigation.handleKeyEvent({ direction: 'UP' })
      expect(navigation.currentFocusNodeId).toEqual('b')
      expect(navigation.getNode('parent').activeChild).toEqual('b')  // Fails: activeChild is 'a'

Expected behavior
In this test case parent's activeChild should still be 'b', 'a' has never been focusable so can't be active.

Moving a node index under the same parent

Is your feature request related to a problem? Please describe.

We are trying to move a node in a list. But we can't call lrud.moveNode with the same parent and a different index, it just doesn't do anything.

Describe the solution you'd like

lrud.moveNode('my-node', 'same-parent-as-before', { index: newIndex })

Describe alternatives you've considered

For now, our idea of a workaround is moving the node to a virtual and unaccessible node at the root of our tree, then moving it back right away in the proper parent with the proper index.
(we haven't tried it yet)

Best way to restore default active child when entering / leaving a parent?

When user navigates down to the player controls on TV I'm wanting it to always be on the play/pause button by default but it's currently just focusing the far left.

I'm not sure indexAlign is what I want for this situation but maybe it is?

I didn't understand the indexRange thing as it doesn't seem to make sense for me here since it's technically not a grid, I mean I can think of it like one... but not sure its very expressive.

UPDATE:
Part of me wants like... a "willActive" or "beforeActive" to ensure the proper child is active but not sure if that makes sense.

UPDATE 2:
If I use onActive of the parent to setActiveChild it doesn't seem to trigger the other events for other children that were previous active such as onLeave or onMove, etc. Maybe I'm missing the entire point of them or something.

Registering a root node if one already exists

To replicate:

const { Lrud } = require('lrud')

const navigation = new Lrud()

navigation.registerNode('root', { orientation: 'vertical' })
navigation.registerNode('root', { orientation: 'horizontal' })

console.log(navigation.tree)

Problem: trying to register a root node again (with the same id, orientation etc doesn't matter) does not change the existing root node to the new one. Instead, it adds another node at the root of the navigation tree, except the node is added with an undefined id.

Expected behaviour: either throw an error to say this node already exists (make it explicit that we can't do it), or unregister existing root node (see other issue I submitted!) and register new one in it's place.

Unregistering root node causes error

To replicate:

const { Lrud } = require('lrud')

const navigation = new Lrud()

navigation.registerNode('root', { orientation: 'vertical' })
navigation.unregisterNode('root') // => ERROR: Cannot read property 'children' of undefined

Problem: Lrud assumes that there's always a parent node of the node we want to unregister. This is an incorrect assumption, preventing us from unregistering a root node.

Problematic line that throws an error: https://github.com/bbc/lrud/blob/master/src/index.ts#L222