ks: section dropdown - #10298
Conversation
|
I am quoting the last feedback from the previous pull request.
|
|
Bwt, I do not know, why the css comes with all these changes again. I used the local sass bin
|
|
Thanks for the fixes! Very much appreciated! SCSS compile is correct. We had one wrong compile in trunk and pieces of it keep coming back from hasty rebases and merges. Should be resolved once this (and/or) my recent PRs are merged. |
thibsy
left a comment
There was a problem hiding this comment.
Hi @alex40724
Thx for implementing the requested changes.
There are some things left to do before we can merge this.
Please consider the following suggestions. You do not need to follow them, but please indicate shortly why you prefer to do otherwise:
- Unit test: you are tightly coupling the rendered HTML of the newly accepted dropdown to this test. We prefer to use rendering stubs to avoid this. Please have a look into our documentation for an example.
Please implement the following changes:
- Union-type: please update the type-hints all the way down to the implementation and properties to get rid of the
instanceofchecks inside the constructor. - Examples: your new examples are missing a description entry, please provide them with one.


This is a follow up of #7837
This PR changes the Section View Control to allow Dropdowns as middle items.

Use cases are navigational elements like here:
https://docu.ilias.de/goto_docu_wiki_wpage_7518_1357.html
Docu Changes:
https://github.com/ILIAS-eLearning/ILIAS/pull/7837/files#diff-4cefcd552bb8e4cb0bf88461b0ea86defb16d82fe86477c3738bc0efcaea6697
Currently the Section View Control documentation includes "Split Buttons" which do not exist, so this part is removed. However the Month Buttons are already allows, so this is added.
This PR already includes an implementation. The correspdoning methods already allow to pass any component, the following check has been extended to check for the Dropdowns.