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

sort_column_toggle is unidirectional #10719

Closed
mixxxbot opened this issue Aug 23, 2022 · 27 comments
Closed

sort_column_toggle is unidirectional #10719

mixxxbot opened this issue Aug 23, 2022 · 27 comments
Labels
Milestone

Comments

@mixxxbot
Copy link
Collaborator

Reported by: robi970
Date: 2022-04-20T15:08:21Z
Status: Fix Committed
Importance: Wishlist
Launchpad Issue: lp1969634
Attachments: [command: sort order](https://bugs.launchpad.net/bugs/1969634/+attachment/5583532/+files/command: sort order)


despite the meaning of the command, with the command "sort_column toggle" in the xml file you can sort the reference column only once, in ascending order.

            <control>
                <group>[Library]</group>
                <key>sort_column_toggle</key>
                <status>0x90</status>
                <midino>0x66</midino> 	
                <options>
                    <Normal/>
                </options>
            </control>		

To alternate between ascending and descending mode, I had to resort to the script:

nameConsolle.sort_column_toggle = function (channel, control, value, status, group) {
if (value == 0x7f) engine.setValue ("[Library]", "sort_column_toggle", 0);
};
(the script works even if you assign the value 1 instead of 0)
@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-04-24T09:17:57Z


This control toggles the sorting of the column specified by the integer argument.
So, if you bind this control directly in the xml it will fire both on press (1) and release (0), thus '0' will toggle the sorting of the Title column, '1' that of the Artist column.

https://manual.mixxx.org/2.4/en/chapters/appendix/mixxx_controls.html#control-[Library]-sort_column
https://manual.mixxx.org/2.4/en/chapters/appendix/mixxx_controls.html#control-[Library]-sort_column_toggle

FWIW I'd appreciate a control that toggles sorting of the column of the currently highlighted table cell.

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-04-24T09:28:29Z


oh, sorry for the noise. I got confused by the fact that this is not working if the Developer Tools Window has focus and that it falls back to interpreting the integer as column id.

If the tracks table has focus it is indeed using the current cell for sorting, though it fires on press and release (tested with keyboard).

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-04-24T09:29:24Z


We could add a wrapper to common-controller-scripts.js to allow simple xml bindings that act only on press.

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-24T15:53:23Z


the Reloop RP8000 turntable taught me that it has buttons that send the midi signal to press and then to release (the 8 drumpads) and buttons that instead send the pressure and release signal both at the moment of release, with an interval of 40 milliseconds (Trax button and function or shift buttons).

For this reason, before reporting the bug, I wanted to check that the xml file did not sort the column twice (once on press and the other on release).
it does not do it:
nothing happens to the pressure of the drumpad;
when the drumpad is released, Mixxx sorts the column.

when I created the script function, however, I initially wrote it:
console name. sort_column_ toggle = function (channel, control, value, status, group)
engine.setValue ("[Library]", "sort_column_ toggle", 0);
and in fact at the pressure of the drumpad Mixxx it ordered the column in one direction,
upon release he re-ordered it in reverse.
in fact then I entered the command "if (value == 0x7f)".

but the xml file works in one way only

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-04-24T17:15:58Z


In your xml example you didn't set a MIDI option, it's default <Normal/>.
How does it behave when you choose 'Switch'? (also try 'Button')
Pref > YourController > Input tab > 'Option' column

I can't test atm, I'm not familiar with the MIDI details and the only documentation is in the Wizard chapter
https://manual.mixxx.org/2.3/en/chapters/advanced_topics.html?highlight=switch%20mode#controller-wizard

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-24T22:08:51Z


summary:
With "normal" it orders creascently only on lifting. when I press, it does nothing

update:
with "switch" the drumpad behaves like this:
when I press, it randomly sorts one way; when I lift, it re-sorts in the reverse random sense.

With "Button":
when I press, order randomly; when I lift, sort (and stays) in ascending order.

it probably behaves like this because, to give the ascending / descending order, Mixxx still waits for the value "0".
every time the drumpad is pressed, the value "1" starts before the "0"
https://manual.mixxx.org/2.3/en/chapters/appendix/mixxx_controls.html?highlight=sort#control-[Library]-sort_column_toggle

but it is also likely that I am missing something

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-24T22:15:28Z

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-04-24T22:28:26Z


what is "random" and "reverse random sense"?
Either items are sorted or not. Only clicking the cover art column should randomize tracks. Are you in that column, or did you click its header earlier?

"every time the drumpad is pressed, the value "1" starts before the "0""

I don't quite understand what you mean.
Button press sends 1, release sends 0

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-25T01:07:14Z


I try to explain myself by entering the description of the command "sort column toggle" from the page manual.mixxx.org/2.3:
«[Library]sort_column_toggle
Equivalent to clicking on column headers. A new value sets [Library],sort_column to that value and [Library],sort_order to 0, setting the same value again will toggle [Library],sort_order.
Range: Same as for [Library],sort_column or value 0 for sorting according the current column with the cursor on it»

the right script function is:
if (value == 0x7f) engine.setValue ("[Library]", "sort_column_ toggle" , 0);


if the value 1 is assigned to the function:
if (value == 0x7f) engine.setValue ("[Library]", "sort_column_ toggle" , 1);
as I mentioned in the first message:
the function works,
but Mixxx sorts the column randomly. That is, if it is positioned on the BPM column, Mixxx will not sort the BPM column in ascending or descending order
(I remember reading the description of "random order" somewhere in the Wiki). But the library lines change position.
By re-pressing the drumpad, the "random" order is reversed (what was the last line becomes the first and vice versa)

when I press drumpad (xml file only, "switch"), while pressed
the value "1" is assigned to "sort column toggle", so Mixxx sorts the column randomly.
when lifting, the value "1" is again assigned to "sort column toggle" and then Mixxx reverses the order of the column.

Xml file only, "Button":
when you press drumpad, "1" is assigned to the "sort column toggle" function and then Mixxx sorts the column at random.
on release, the value "0" is assigned, then Mixxx sorts the selected column in increasing order.

Only xml files, "Normal":
pressing the drumpad to "sort column toggle" is assigned a value that does nothing, so Mixxx does nothing.
On lifting, the value "0" is assigned to the "sort column toggle" function and then Mixxx sorts the selected column in ascending order

in order to alternate between ascending and descending sorting, the value "0" must be sent to the "sort column toggle" function twice in a row (this is how the script function works),
but none of the three functions (normal, switch or button) is able to send the value "o" to "sort column toggle" without first sending something else, so "sort column toggle" never switches from ascending to decreasing

mmm ... I'm doubting that the assignment of the value "1" to the "sort column toggle" function will sort the "artist" column, because "1" is the value of the Artist column in the "sort column" tabel

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-25T01:48:18Z


I tested the doubt that had arisen and xml file works like this:
"switch" assigns "1" to "sort_column" both in pressure and in lifting, then sorts the Artists column in ascending and descending order; "button" pressin drumpad it assigns "1" to "sort_column", then sorts the Artists column in ascending order, while in lifting it assigns "0" to "sort column toggle", then sorts the selected column in ascending order;
"normal" I don't know what it assigns under pressure, but nothing happens, while on lifting it assigns "0" to "sort column toggle" which sorts the selected column by ascending (but does not change to descending when the button is re-pressed).

in the library I don't show the artists column so I didn't notice it before

attention: I was not wrong to mention "sort column" and "sort column toggle"

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2022-04-25T07:44:39Z


Maybe looking up the C++ helps:

m_pSortColumnToggle = std::make_unique<ControlEncoder>(ConfigKey("[Library]", "sort_column_toggle"), false);

void LibraryControl::slotSortColumnToggle(double v) {

"sort_column_toggle" is designed to be mapped to an encoder knob.
You can scrolls to the columns and sort them, where 0 is the current column, and Artist = 1, Title = 2 ...

If you just like to toggle the sort order via a pushbutton,
"sort_order" can be used

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2022-04-25T07:49:52Z


I did not understand which function you like to achieve exactly. Maybe there is a missing bit in our C++ code.

Can you describe the user action on the controller and the expected behaviour of Mixxx without implementation details?

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-25T12:21:11Z


Daniel, I thought about reporting the bug when I realized that the "sort column toggle" command in the xml file did not work both ways (ascending and descending), but only sorts the column in ascending order.
I didn't understand why a command, specific to toggle ("_toggle"), had to be set via the script.

your last message made me understand that perhaps the command was not born to be set in the xml file, but is specific for the script.

I set up Mixxx like this:
using the Trax knob I scroll the row, using a drumpad I order the desired column.
but until now I believed that "sort column toggle" should only be used to sort the selected column
and that instead, to sort a specific column, there was the command "sort column"

practically:
if "sort column toggle" was not born as a command for the xml file, I have reported a bug that is not there.
in case I apologize for wasting your time reading messages written with google translator, which I realize is sometimes really difficult to interpret

PS:  the command
engine.setValue("[Library]","sort_column",15);
sorts the BPM column in ascending order, but does not switch to descending when re-pressed.
for this reason I thought that "sort column toggle" was specific to the selected column

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-25T13:01:33Z


the function:
            <control>
                <group>[Library]</group>
                <key>sort_order</key>
                <status>0x90</status>
                <midino>0x66</midino> 			
                <options>
                    <normal/>
                </options>
            </control>

alternates the ascending and descending order,
but not that of the box selected by the Trax knob, but remains blocked on the column preset by Mixxx, or the last one sorted by the mouse (the one in which the symbol: "^" is highlighted)

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-04-25T13:51:02Z
Attachments: [command: sort order](https://bugs.launchpad.net/mixxx/+bug/1969634/+attachment/5583532/+files/command: sort order)


in this video you can see how the "sort_order" function works in the xml file ..

with the mouse I sort the "year" column so that this is the column that will remain preset (not "preview").
then I move with the trax knob to BPM, but when I press the "sort order" button to sort it, Mixxx reverses the order of the "year" column.
then I move with the trax konb on the "time" column, but when I press the drumpad "sort order" Mixxx continues to reverse the order of the "year" column

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2022-05-09T20:29:46Z


How do does your Trax knob move to BPM? Which Mixxx control is involved?
I guess it is MoveLeft / MoveRight.

This meas we have to change the implementation that the [Library],sort_order will also change the sort column to the selected column. That sounds reasonable.
I will give it a try.

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-05-09T22:29:56Z


It's already working like this.

void LibraryControl::slotSortColumnToggle(double v) {
int sortColumnId = static_cast<int>(v);
if (sortColumnId == static_cast<int>(TrackModel::SortColumnId::CurrentIndex)) {
if (!m_pLibraryWidget) {
return;
}
// Get the ID of the column with the cursor
sortColumnId =
static_cast<int>(m_pLibraryWidget->getActiveView()
->getColumnIdFromCurrentIndex());
}
if (static_cast<int>(m_pSortColumn->get()) == sortColumnId) {
m_pSortOrder->set((m_pSortOrder->get() == 0) ? 1.0 : 0.0);
} else {
m_pSortColumn->set(sortColumnId);
m_pSortOrder->set(0.0);
}
}

It's just that sort_column_toggle is simply not designed to be mapped to pushbuttons (xml or GUI) that send 1 on press and 0 on release.
When I link a 2-state GUI pushbutton to sort-column_toggle it's obvious:

  • press always sorts by artist (columnId = 1)
  • release sorts the column of currently focused cell (columnId = 0, so
    the focused column is sorted)

We could simply add a wrapper control 'sort_focused_column' that acts on press only (v > 0) and calls slotSortColumnToggle(0).
And make sure sort_column_toggle is not in the MIDI wizard menu to avoid confusion

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-05-10T11:53:33Z


#4749

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-05-10T13:24:12Z


"How do does your Trax knob move to BPM? Which Mixxx control is involved?
I guess it is MoveLeft / MoveRight."

that's right, Daniel, it's "MoveLeft / MoveRight"

@mixxxbot
Copy link
Collaborator Author

Commented by: daschuer
Date: 2022-05-10T14:39:16Z


We could simply add a wrapper control 'sort_focused_column'.

Yes, I had a look how to reuse the existing controls, but that would be rather unpredictable.

This is the PR:
#4749

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-05-10T16:44:28Z


currently Mixxx uses many commands to do almost the same thing (move between folders, files and track data).
This implies that if I want to manage the Mixxx through a console, the console should have many buttons and traxes dedicated to just that.

I would like to propose to create a command that brings together as many functions as possible:
through a trax (or by pressing a key several times), you can change focus (move_focus), but not only "resources" <==> "library", but be able to focus every part of Mixxx (deck1, deck2, settings, master ...).
each part, when selected with the "GoToItem" button, can in turn be navigated through the usual trax or button and you can still select the object using the "GoToItem" button/command, or return to the upper level with the button/command (example:) "return".

through a drop-down menu (for example in the settings), list all the parts of Mixx that can be focused via trax or button, being able to flag them in order to eliminate the parts in which you do not want to navigate, including the submenus (at inside Deck1, to be able to flag "hotcue" so you can use them via "GoToItem", but maybe not flag "beatjump" so as not to waste time scrolling through the parts I know I never want to use via "GoToItem").

in this way, for example, I could choose, with the trax or the button, to navigate inside deck1 to scroll to the "Hotcue3" button
and set it using the "GoToItem" button; or set through "GoToItem" the command "loop_activate" of Deck2 ...
if I focus on the library, the trax or the button would scroll the columns .. etc

in this way, however, "GoToItem" could no longer be used to load a track from the library, because when pressed it would put the column in focus in ascending / descending order

keep in mind that more and more consoles come out with 6/8/.. settable midi buttons (+ "shift" buttons) and maybe even with trax
(I hope google translate did its job well)

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-05-11T10:14:46Z


this bug was just fixed by adding [Library], sort_focused_column and it was merged to the 'main' branch. So the fix is available in the nightly builds (ppa) / development snapshots (mixxx.org/download).

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-05-11T10:14:59Z


re your unrelated feature request: you're asking to

  • make every GUI control focusable
    = rework most Mixxx widgets
    = add highlight styles for every widget
  • put them into a reasonable order (i.e. different from the underlying skin structure) PER SKIN
  • add mechanics to skip widgtes, add basically ALL widgets to a preferences list

It's unclear if all this is possible, and if it is that is an isane amount of work, and it is quite the opposite of the current design direction.
Let alone the UX: use an encoder to cycle through all widgets? This is incredibly cumbersome, reminds me of programming DVS recorders and alike back in the 90s.

Bottom line:
highly unlikely someone will even think about the implementation details.

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-05-11T15:26:30Z


I feared the difficulty and I understand.

I made that proposal starting from the assumption described in the "moveFocus" command:
«[Library] MoveFocus
Move focus the specified number of panes forward or backwards. Intended to be mapped to an encoder knob. "

currently "moveFocus" acts only in a binary way:
shifts the focus on resources,
brings the focus back to the tracks.
As it stands, it makes no sense to predict its use with a knob.
And dedicating an exclusive button to it requires that the console in use is equipped with it, otherwise it would be a wasted "generic button".

my previous proposal was in search of the "optimum", but it could be revisited by making the "MoveFocus" command (or the "moveLeft" and "moveRight" commands) assume the possibility of bringing the focus also between the various columns of the tracks.
that is through the command "moveRight" (or "moveFocus") to be able to pass from the "resources" section, to the first column of the library, to the second, to the third .. etc and then to return to the "resources" section and close the circle.
"the" moveLeft "command would do the same path, but backwards.

practically:
delete the "moveFocus" command,
or delete the commands "moveLeft" + "moveRight", as they are no longer necessary

@mixxxbot
Copy link
Collaborator Author

Commented by: ronso0
Date: 2022-05-11T22:58:08Z


shifts the focus on resources,
brings the focus back to the tracks.

It would also focus the searchox' Clear button if it's not empty.
Also, consider this control 'futureproof' for when there will be multiple track views side.

IMO merging 'MoveRight' and 'MoveFocus..' commands means that getting back from the table to the sidebar requires scrolling through all columns left of the current table cell. The other way around, going from the sidebar to the tracks table would always reset the horizontal scroll position to the leftmost column.
This is an inacceptable UX regression, at least for me. Most controller have a rotary pushbutton encoder as well as Shift button. With those two you can script pretty much any library navigation pattern you like, considering [Library],focused_widget was added to Mixxx 2.4 (still alpha).

https://manual.mixxx.org/2.4/en/chapters/appendix/mixxx_controls.html#control-[Library]-focused_widget

@mixxxbot
Copy link
Collaborator Author

Commented by: robi970
Date: 2022-05-12T20:58:03Z


"IMO merging 'MoveRight' and 'MoveFocus..' commands means that getting back from the table to the sidebar requires scrolling through all columns left of the current table cell. The other way around, going from the sidebar to the tracks table would always reset the horizontal scroll position to the leftmost column."

right. :)
but it must be taken into account that the consoles do not have all these dedicated buttons or to be dedicated (I am not talking about my RP8000 because after you have shown me how to use the same pad for two different commands, I can play on 144 pads), so it turns out that no one will ever implement them ...
also, with generic pads this could happen:
"how was it? ... the first pad is to move the focus and the second to move between the columns? .. or maybe I had set the third pad to scroll through the columns and the first to ...?"
and in the end, even if set, the pads would never be used to move around the library because it would be a very slow action.

instead, consider:

  • "moveLeft" & "moveRight" (or "MoveFocusForward" & "MoveFocusBackward") to scroll through "preview", "resources" & "every single column of the tracks" (to be set on the knob, so you can quickly scroll everywhere)
  • "moveFocus" use it only to move the focus as it is now (preview, resources, library)
    how would you see it?

@mixxxbot
Copy link
Collaborator Author

Issue closed with status Fix Committed.

@mixxxbot mixxxbot transferred this issue from another repository Aug 24, 2022
@mixxxbot mixxxbot added this to the 2.4.0 milestone Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

1 participant