CogentRedTester / mpv-scroll-list

MIT License
36 stars 3 forks source link

chapter-list: refactored and added new features #4

Closed dyphire closed 2 years ago

dyphire commented 2 years ago

optimize scroll-list.lua:

refactor chapter-list.lua:

dyphire commented 2 years ago

@CogentRedTester Do you have any opinion on this PR? I also created several new scroll menu scripts based on the new features. see adevice-list.lua, track-menu.lua, editions-notification-menu.lua

dyphire commented 2 years ago

In addition, I also found a problem that the list.selected will not be reset after switching files, resulting in its wrong state.

At present, I assign a new value to it to solve this problem. Do you have a better idea?

dyphire commented 2 years ago

I have another question related to the above. When opening the scroll menu for the first time, the cursor stays on the currently used list item. At this time, move the cursor to other list items at will and close the menu.

After opening the menu again, the cursor at this time will inherit the state before closing. How to reset the cursor state to ensure that each opening stays on the currently used list item?

dyphire commented 2 years ago

How to reset the cursor state to ensure that each opening stays on the currently used list item?

Reference code from https://github.com/jonniek/mpv-playlistmanager/commit/b3b75829bb77661370dddea929deccda89f59c71 , implement the cursor-pos reset function for chapter-list.lua.

As with reset function of list.selected after switching files, neither seems can be implemented in scroll-list.lua.

CogentRedTester commented 2 years ago

How to reset the cursor state to ensure that each opening stays on the currently used list item?

Reference code from jonniek/mpv-playlistmanager@b3b7582 , implement the cursor-pos reset function for chapter-list.lua.

As with reset function of list.selected after switching files, neither seems can be implemented in scroll-list.lua.

Yes those aren't and shouldn't be added to scroll-list. We have no way of knowing what scroll-list will be used for, it's up to the scripts using scroll-list to implement those features as you have done in chapter-list.

CogentRedTester commented 2 years ago

Also change the commit message to chapter-list: refactored and added new features and include at least a basic list of the new features added (key options, new selector behaviour, etc)

dyphire commented 2 years ago

Also change the commit message to and include at least a basic list of the new features added (key options, new selector behaviour, etc)chapter-list: refactored and added new features

Attempted to rewrite the commit message

dyphire commented 2 years ago

@CogentRedTester Do you have any views on the current changes?

CogentRedTester commented 2 years ago

Looks good, I'll merge when you're ready.

dyphire commented 2 years ago

Looks good, I'll merge when you're ready.

If you have no comments on the commit message, then I am ready.