-
Notifications
You must be signed in to change notification settings - Fork 344
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
UI 42336: Fixes being unable to sort table records by columns that are not show… #8222
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @matheuszych ,
I've been pondering about this for a bit; as you figured, the ViewControls are interdependent and have to be re-initialized.
For the interested readers of this: FieldSelection provides the selected columns, and Sortation reads $table->getVisibleColumns.
When retrieving the Viewcontrols here, the table is not yet updated.
What happens (and why this is a good solution): first call on the controls is just to retrieve container-data from the request, while the second call - provided with this PR -is to actually get and display the ViewControls.
I was a little unsure at first because of (my own) naming and the seemingly redundant call; maybe it will also help others if we made this a little more obvious, e.g. by
$data = $this->applyValuesToViewcontrols($view_controls, $request)->getData();
instead of setting $view_controls in l128 already?
@matheuszych , would you be so kind as to bear with me and change those lines, too, perhaps even add a comment?
Anyway, thank you a lot, and best regards,
Nils
Hello @nhaagen, |
Thank you very much, @matheuszych |
Thanks! |
Picked to release_8 and release_9. |
When trying to sort the entries of a table using the viewcontrols you get only shown the columns that are visible by default. Even when enabling optional columns you are unable to sort by them because the according entries in the dropdown menu are missing. This applies to the new KS tables.
I believe this could be a possible fix.
Mantis