SlimBrowse: nextWindow is ignored about lms-material

[CDrummond]

Hmmm... I have no way of checking this, as I don't see these. However, I have just added some code to check 'nextWindow' when a 'text' item is clicked. Can you checkout the 58-nextWindow branch, and see if this helps with this?

[michaelherger]

Unfortunately this doesn't really cut it: first of all the condition doesn't cover this case. The menu item's type is not text, actually it's not defined. Then we need to restore the previous menu's state in order to refresh it. So I changed it this way:

diff --git a/MaterialSkin/HTML/material/html/js/browse-page.js b/MaterialSkin/HTML/material/html/js/browse-page.js
index 65563e8..4248a99 100644
--- a/MaterialSkin/HTML/material/html/js/browse-page.js
+++ b/MaterialSkin/HTML/material/html/js/browse-page.js
@@ -593,8 +593,8 @@ var lmsBrowse = Vue.component("lms-browse", {
                 }
                 return;
             }
-            if ("text"==item.type) {
-                if (allowTextClick) {
+            if ("text"==item.type || (null==item.type && item.actions && item.nextWindow)) {
+                if (allowTextClick || "text"!=item.type) {
                     var command = this.buildCommand(item);
                     command.params.forEach(p => {
                         command.command.push(p);
@@ -603,10 +603,14 @@ var lmsBrowse = Vue.component("lms-browse", {
                         if (item.nextWindow) {
                             if (item.nextWindow=="parent" && this.history.length>0) {
                                 this.history.pop();
+                                this.command = this.history[this.history.length-1].command;
+                                this.items = this.history[this.history.length-1].items;
                                 this.refreshList();
                             } else if (item.nextWindow=="grandParent" && this.history.length>1) {
                                 this.history.pop();
                                 this.history.pop();
+                                this.command = this.history[this.history.length-1].command;
+                                this.items = this.history[this.history.length-1].items;
                                 this.refreshList();
                             }
                         }

I probably don't know what I'm doing here :-). I'm not sure that's all the state we'd need to restore, but this got it working.

[CDrummond]

Try again?

[michaelherger]

Something's still not quite right: it would still send the command to delete the item twice. First when it's told to do so, then again in goBack(). I tried adding this.history.pop() before the call to goBack() - which fixed the refresh, but messed up the stack. Going back one more level would not bring me back to the Spotify main menu, but out of Spotify.

I think the problem lies in the fact that goHome() would use the popped item's information to refresh the list instead of the then top most item on the stack. In my patch above I'd throw away the popped item and refresh based on the last command/items in the list.

[CDrummond]

Hmmm... But the popped item should be the one that produced the list. My understanding of what should be happening is:

1. Some JSONRPC command has produced a list of searches that can be deleted
2. Press 'More' button, this places the list from 1 in history, and produces the option to delete
3. Select 'delete' - no new list is fetched, as its just a command
4. Now the goBack() should take the last item off the list, which should be the items from 1 (as well as the command that was used to create the list), It then refreshes with the command used to produce 1

goHome does not refresh the list, so will not activate any command - I take it you mean goBack Using the item in the history list to populate is wrong. this.current should contain the item that was clicked to produce the list shown - i.e. its command/params are what are needed to draw the visible list. So when goBack() pops an item of the history, it sets this.current

Can you create a list of JSONRPC calls, and their responses, used to get to, and calling of, the delete item? I can then see if I can hack this into the code to fake requests/responses - and see the full call trace, etc. This way I can fix the issue by seeing the real results - as opposed to blindly guessing as to what should happen.

[michaelherger]

The popped item would be the little menu offering deletion, NOT the search menu. Stack would be:

     Delete Search Item
  Search

Spotty
Main Menu

Popping that last one off the stack and re-freshing it would be trying to refresh the "Delete..." item instead of "Search". If you look at my patch you'd see that I'd pop the last item, throw it away, then refresh on the top most item.

I'll see whether I can grab the full list of commands/results.

[CDrummond]

Is 'Delete Search Item' the item that sends the 'delete' command, or is it the one that has the 'More' command?

If 'Delete Search Item' is the one that sends the command, it should not be added to the stack.

You should be able to use the browser's dev tools to get requests/responses.

[CDrummond]

Also, just had a quick test - and goBack() seems to be broken. If I get it to call refreshList in all cases, it does not refresh correctly. This implies it is not correctly setting the state. I'll look into this later. This is what I think the real issue with my change is.

[michaelherger]

Is 'Delete Search Item' the item that sends the 'delete' command, or is it the one that has the 'More' command?

When you press More you'll get a new menu which would send the delete command. "Delete Search Item" is that additional menu.

If 'Delete Search Item' is the one that sends the command, it should not be added to the stack.

It is a menu, and its menu items would send delete commands (all or current entry). The deletion action itself is not put on the stack, but this little menu is.

[CDrummond]

Ah, so:

List of search entries, each has a "more" button
Selecting "more" produces menu (list for material) with "Delete Search Item"
Selecting "Delete Search Item" triggers delete command, and should go back to "list of search entries"

In this scenario the list that has "Delete Search Item" should not be on the stack, as its the current list. It should only be put on the stack if it was to be replaced by another list (going forwards). The "Delete Search Items" should not fetch another list, so should not add to stack.

[michaelherger]

Ok, here's the communication. Bring up the search menu:

Request:

{
	"id": 1,
	"method": "slim.request",
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"items",
			0,
			100,
			"menu:spotty",
			"item_id:0"
		]
	]
}

Response:

{
	"result": {
		"base": {
			"actions": {
				"more": {
					"window": {
						"isContextMenu": 1
					},
					"player": 0,
					"cmd": [
						"spotty",
						"items"
					],
					"itemsParams": "params",
					"params": {
						"menu": "spotty"
					}
				},
				"play": {
					"params": {
						"menu": "spotty"
					},
					"itemsParams": "params",
					"nextWindow": "nowPlaying",
					"cmd": [
						"spotty",
						"playlist",
						"play"
					],
					"player": 0
				},
				"playControl": {
					"player": 0,
					"cmd": [
						"spotty",
						"items"
					],
					"window": {
						"isContextMenu": 1
					},
					"params": {
						"_quantity": "100",
						"menu": "spotty",
						"item_id": "0",
						"_index": "0"
					},
					"itemsParams": "playControlParams"
				},
				"go": {
					"cmd": [
						"spotty",
						"items"
					],
					"itemsParams": "params",
					"params": {
						"menu": "spotty"
					}
				},
				"add": {
					"itemsParams": "params",
					"params": {
						"menu": "spotty"
					},
					"cmd": [
						"spotty",
						"playlist",
						"add"
					],
					"player": 0
				},
				"add-hold": {
					"params": {
						"menu": "spotty"
					},
					"itemsParams": "params",
					"player": 0,
					"cmd": [
						"spotty",
						"playlist",
						"insert"
					]
				}
			}
		},
		"title": "Search",
		"offset": 0,
		"window": {
			"windowStyle": "text_list"
		},
		"count": 2,
		"item_loop": [
			{
				"input": {
					"softbutton2": "Delete",
					"processingPopup": {
						"text": "Searching..."
					},
					"len": 1,
					"softbutton1": "Insert",
					"help": {
						"text": "Use the scroll wheel to change letters, then press the center button to select that letter. Press the center button again to start your search."
					},
					"title": "New Search"
				},
				"actions": {
					"go": {
						"params": {
							"item_id": "0.0",
							"menu": "spotty",
							"search": "__TAGGEDINPUT__",
							"cachesearch": 1
						},
						"cmd": [
							"spotty",
							"items"
						]
					}
				},
				"text": "New Search",
				"type": "search"
			},
			{
				"addAction": "go",
				"type": "link",
				"text": "peter gabriel",
				"actions": {
					"more": {
						"params": {
							"deleteMenu": 0,
							"menu": 1
						},
						"cmd": [
							"spotty",
							"recentsearches"
						],
						"player": 0,
						"window": {
							"isContextMenu": 1
						}
					},
					"go": {
						"cmd": [
							"spotty",
							"items"
						],
						"params": {
							"item_id": "0.1",
							"menu": "spotty"
						}
					}
				}
			}
		]
	},
	"method": "slim.request",
	"id": 1,
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"items",
			"0",
			100,
			"menu:spotty",
			"item_id:0"
		]
	]
}

Select More item on the recent search item. Request:

{
	"id": 1,
	"method": "slim.request",
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"recentsearches",
			0,
			100,
			"deleteMenu:0",
			"menu:1"
		]
	]
}

Response:

{
	"id": 1,
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"recentsearches",
			"0",
			"100",
			"deleteMenu:0",
			"menu:1"
		]
	],
	"result": {
		"count": 2,
		"item_loop": [
			{
				"nextWindow": "parent",
				"text": "Delete: \"peter gabriel\"",
				"actions": {
					"go": {
						"cmd": [
							"spotty",
							"recentsearches"
						],
						"player": 0,
						"params": {
							"delete": 0
						}
					}
				}
			},
			{
				"actions": {
					"go": {
						"params": {
							"deleteAll": 1
						},
						"player": 0,
						"cmd": [
							"spotty",
							"recentsearches"
						]
					}
				},
				"text": "Clear complete search history",
				"nextWindow": "grandParent"
			}
		],
		"offset": 0
	},
	"method": "slim.request"
}

Select Delete: "Peter Gabriel". Request:

{
	"id": 1,
	"method": "slim.request",
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"recentsearches",
			"delete:0"
		]
	]
}

Response:

{
	"method": "slim.request",
	"result": {},
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"recentsearches",
			"delete:0"
		]
	],
	"id": 1
}

And then immediately there's this Request:

{
	"id": 1,
	"method": "slim.request",
	"params": [
		"ac:bc:32:ef:ae:d1",
		[
			"spotty",
			"recentsearches",
			0,
			100,
			"deleteMenu:0",
			"menu:1"
		]
	]
}

To which there's no response as the request fails. That search item doesn't exist any more.

[CDrummond]

Thanks for these, I'll see if I can somehow inject these into the code.

In the meantime I have fixed adding the current command to the history, so now goBack() works as expected. This has been broken for quite a while, obviously no-one has been scrolling a list (so that more items are fetched) after going back up a level.

Can you check-out this branch and try again?

[michaelherger]

I think this time you nailed it. Woot! Thanks a lot!

[CDrummond]

Great! I'll close then. Thanks for the confirmation.