T&A 41371: Fix competence path not displaying properly

[bidzanaaron]

This PR aims to address the problem reported in Mantis issue https://mantis.ilias.de/view.php?id=41371.

When assigning one or multiple competences to a question in a test, the root node of the competence isn't properly displayed. To provide better readability, I have added language variables in both German and English to replace the root node title accordingly.

Adding onto that, I have also removed a CSS selector that added padding, which made the first competence not align with the others.

Kind regards.

[catenglaender]

Thank you for tackling this issue. :+1: As far as styling goes, there are some guideline best practices that you must comply with before this can be merged. It looks like you edited the .css file directly. Please note, we are using a pre-processor that compiles .scss files into .css, which will override your manual change. Here are some relevant parts quoted from templates/Guidelines_SCSS-Coding.md

• Style code MUST be written in the SCSS syntax of the Sass pre-processer. [...]
• When contributing style code to the ILIAS repository, you MUST first compile the Sass code to CSS using the latest released version of Dart Sass: https://github.com/sass/dart-sass/releases [...]
• You MUST NOT make changes to the compiled delos.css manually.

The part you aimed to remove here is actually located in /templates/default/070-components/legacy/Services/_component_table.scss

However, before removing it you might want to research e.g. using git blame (maybe even in release_8, searching through the old less files) why this was done in the first place and check if it is still needed for what this line was originally written for. This is quite a general line affecting all legacy tables, so we need to be careful that this won't cause 10 new issues. :wink:

Personally, I am all for getting rid of special/exception paddings if you can confirm that it is no longer needed elsewhere. :slightly_smiling_face:

Feel free to ask me, @lukastocker or @Amstutz if you have any questions left after digging into the SCSS guidelines/documentation and the history of this pesky padding you are removing.

Thanks again and have a fantastic week!

[bidzanaaron]

Since there is a general issue with the display in relation to rowspan and :first-child, we have decided to achieve the said goal with local changes instead of the changes that I have provided before.

[thojou]

Hey @catenglaender,

As @bidzanaaron mentioned, we've decided to revert the styling changes. After further investigation, we found that fixing this specific table representation with custom CSS rules is challenging and would likely impact existing designs.

Instead, we applied a workaround to adjust the current CSS rules appropriately without modifying any SASS/CSS code.

@kergomard, @mbecker-databay, are we okay with using a hidden cell to fix the representation? This may be reworked during the UI-DataTable refactoring anyway.

[dsstrassner]

Why is this merged without the Feedback from @oliversamoila and me?

[oliversamoila]

So for me, it was probably the speed and the effort that I was able to spend on this topic. 😄 I wish I'd have had more - of both.

[catenglaender]

Hey everyone, thanks for the update @bidzanaaron

Just to be very precise: Adding "display: none;" as an inline style attribute is explicitly disallowed with "MUST NOT" in the SCSS style guide. I know there is old code that has this, but we must get rid of this in the mid-term. Please try to never ever do this anywhere else.

But... ooookay... It's localized to this one very specific legacy table here. I guess if this PR absolutely must be brought over the finishing line limping like this, so be it. UI Data Table Refactoring would be oh so nice.

Can you give a quick hint at why this first-child styling is actually needed? Maybe we can tackle why on earth the table needs this... but then again it's legacy table, so hopefully it's a problem that simply goes away if more and more tables transition to the UI Data Table.

[dsstrassner]

HI @maxbecker & @thojou, please revert this PR and check with @catenglaender to solve this issue properly. As ILIAS 9 is there until 2027, we need a solution that fits his wishes and what is stated in the guidelines and helps the users.

Also, @oliversamoila stated that he sees no changes on test9 through this PR.

[thojou]

Hello @dsstrassner, I've reverted the PR to continue discussing the implementation details.

Hello @catenglaender,

Thank you for your detailed response. I also prefer a solution without using display: none; inline.

From my understanding, the extra padding is applied to each first column of the table, which is used for the multiple select checkboxes.

In the table containing the issue, there are no checkboxes in the first column, so the extra padding is applied to the content cells instead. This issue escalates because we use a table column with rowspan definitions. This causes the browser to render the same column over multiple rows while still detecting the secondly displayed columns as the first-child from the second row onward.

I created a JSFiddle to describe the issue in an isolated environment: https://jsfiddle.net/wx9jye7v/10/.

[catenglaender]

Thanks for the detailed investigation! Since you are so close to the root of this problem, I wonder if you can actually fix the core of it. How about adding a left margin to checkboxes inside the legacy table instead of the padding to the cell? This should have visually the same result, but only apply to checkboxes.

Thanks for your patience and sorry for the inconvenience this minor detail is causing. I would feel a lot more comfortable with shifting the exception styling away from the cell and onto the checkbox.