ILIAS
ILIAS copied to clipboard
TA: Vertical drag and drop order question design 33651
Mantis issue: https://mantis.ilias.de/view.php?id=33651
Issue
Vertical and horizontal drag and drop elements in order questions look significantly different. It seems that style code from the dependency causes an unwanted inner block design.
Changes
Two challenges made this fix interesting:
- The drag and drop dependency used (Nestable 2) brings a lot of unwanted style code with the default CSS classes it expects for various elements (e.g. "dd-handle")
- there is a lot of PHP involved to assign the class names of the HTML elements rather than just writing them directly into the HTML templates.
To get the desired result I made the following changes:
- The CSS Handle class had a getter and a setter, but wasn't actually filled into the JavaScript, so I made the placeholder tag work inside the JS config of the drag and drop as well
- With a value for the CSS Handle Class actually rippling through, I could change the name of the CSS class from dd-handle to il-dd-handle which has no styling from the dependency associated with it. I left the names of other elements unchanged as their styling is not visual but important for the drag and drop functioning correctly.
- to avoid a bug from the dependency with multiple classes for the handle div class, I nested the ilc_qanswer_Answer class in its own div div inside the handle div. This is how the horizontal ordering quiz is set up as well.
- there was one font-size definition left over coming from the dependency that I had to override in the Test delos.less
More could be done
I wonder if this whole system should be refactored. Wouldn't it be much easier to simply write most of the classes plainly inside the the HTML templates without using placeholders? What is the common best practice here?
Outside of actual dynamic values like the ID, nested layer depth, and there is an ol/ul switch (which I am not sure we ever make use of) I do not quite see the need for it being so flexible, especially because that flexibility doesn't seem to be in use and is just filled with default values.
And maybe the horizontal and vertical drag and drop order questions could use a single dependency that supports both directions?
Unfortunately, both of these tasks will probably be quite laborious and might effect certain possible question configurations that I may not have an eye on. So that's why I choose this more minimal patch for now. If this spot is a priority or bigger changes are planned for this question type, we might want to revisit this.
I am good with this one, thank you so much!
Screenshots
Drag no nesting
Drag with nesting
Hi @catenglaender
Thx a lot for this proposal. The CSS change looks fine for me. However, since we merge that into the productive 7 release, did somebody test this throughly? I could but, than I would need somewhat more time. With DOM changes I am always somewhat catious in TA due to not intended side effects on a JS level... If so, IMO this can be merged and picked to 7.
Concerning your questions:
Wouldn't it be much easier to simply write most of the classes plainly inside the the HTML templates without using placeholders? What is the common best practice here?
Completely agree. Not sure if there is a common practice in legacy html. IMO it is definitely bad practise to use place holders for classes used for styling. Designers are completely locked out this way. In UI Components we do no allow this. We should get rid of it here as well. However, I would not invest to much ressources in this atm, since we probably move lots of the rendering at some points to the UI Components anyway.
And maybe the horizontal and vertical drag and drop order questions could use a single dependency that supports both directions?
Sound right.
Thx for this.
I have tested creation and the use of the changed ordering questions. So far, everything seems to work. (in Pools & Tests) But I am not happy with the styling in the nesting area of the creation process.
I will discuss this with @catenglaender after my vacation. So this one needs a little bit longer ;-)
@dsstrassner Thx a lot for looking into this. I de-assigned myself ftm. Please don't hesitate to re-assign as soon as I can contribute something further here.
Content Styles now load during editing of nesting order
The reason why the vertical drag and drop question appeared unstyled is because class assOrderingQuestionGUI did not load the content styles during the cmd editNesting. This has now been fixed.
Slight visual update
Nesting looks like this now with borders making the parent-child pairing even more clear and aiming a bit easier:
Some questions remain
Why are content styles loaded that selectively? Shouldn't the general assQuestionGUI always load the content styles instead of this being in the hands of a very specific command? When do we ever want to display questions without the content styles? Is that because of that print and result view?
I have tested the new Style in @catenglaender branch. With the style is everything fine. But the Nesting Order is reset, if someone changes setting on the Edit Question Subtab (even no changes in Answer items, e.g. only the description is changed). I tried to test this behavior on test7, but there the nesting is not saved ... (Ping @nhaagen).
Also, I assigned @kergomard and @Amstutz - perhaps they can answer your question @catenglaender.
Thanks for this PR from my side, too! I can actually not really answer your question without looking further into it (and this will have to wait for the moment). My guess would be that the test should centralize loading of the styles, but that for some historical reason it isn't. But as said: This is just educated guessing.
At this point, I suspect one of the main reasons why the content styles are loaded selectively might be the print view. If we load the content styles with the global question rendering, the print view has them too, but that didn't seem to be desired. However, I think this potentially leads to bugs as it's easy to forget to bring style changes into the print view as well. Some questions (like kprim, or the vertical ordering) absolutely need styling to not look broken.
Unfortunately I have no answer to the content style related question. @dsstrassner Maybe discuss in the squad on how to proceed here?
Thanks @Amstutz. Yes, we discuss this pull request on the next TechSquad Meeting 2022/12/15.
I have forgotten to comment, that I decided at the TechSquad Meeting (15.12.12), that @kergomard will merge this PR and cherry-pick accordingly to ILIAS 8 & Trunk.
@catenglaender : Sorry this go stuck with me for so long! Could you please rebase and force push? I tried to do this through the webinterface, but apparently that doesn't work, sorry. I will then merge it quickly.
Rebased PR on top of current release_7. Merge should be possible now.
Thanks @catenglaender / happy merge @kergomard 😄
Thank you very much @catenglaender ! Will merge and cherry-pick to 8 and trunk.