ILIAS
ILIAS copied to clipboard
UI/ViewControls/Sortation: 26634, mark selected option
https://mantis.ilias.de/view.php?id=26634 origin: https://github.com/ILIAS-eLearning/ILIAS/pull/6026
Hi @nhaagen, thank you very much for your work on this issue. Do I understand correctly that this PR directly suggests changing the existing usages of the View Control Sortation? That would be wonderful if the many different solutions were to appear in the same look and feel.
Hi @oliversamoila,
yes, this would adjust usage locations as well to streamline the thingy.
Kind regards!
@kergomard please ping me on Discord about this PR on short notice.
About the hidden label: As far as I know, there are no clear guidelines or rules yet on how to handle "rendering" a variation of an UI component and therefor no clear alternative to setting flags that trigger custom non-default designs outside of php. This is definitely a topic we should discuss at next week's CSS Squad, @Amstutz
Our BEMIT naming convention for SCSS technically has a way to depict style variations:
il-viewcontrol-sortation
could be additionally given a variant class like il-viewcontrol-sortation---no-label
. This would be a valid way to trigger an out of the default styling according to our guidelines - but the question remains of how this class name got there.
On pretty much any other project that I have worked on I would heavily advocate for the style change coming from the context and "detect" that with CSS classes. Something like this:
.il-dashboard { // the context in which the viewcontrol should appear differently
> .panel .viewcontrol-container .il-viewcontrol-sortation { // only change the appearance of this very specific viewcontrol
label { display: none; }
}
}
The problem is: In ILIAS, such context clues (like .il-dashboard
) where exactly the component is being rendered are almost always missing. Often we cannot detect via CSS if this is this exact viewcontrol in this specific view or in which panel of many we want to force the change from the default. And as far as I understand, fetching context information and adding it to the page container or panel as a css-class name is not really an easy feature for ILIAS.
And last but not least we do have design changes that actually might need structural changes in the DOM (with modern CSS grids and flex-box this is less and less necessary, but it still happens) - e.g. maybe if the label is missing, more aria-labels have to be set or an entirely different html template has to be used. I don't think this could ever be handled outside of php.
This is a lot of words for: I don't know yet what the best solution would be. From my gut feeling I feel like we are a bit away from always avoiding design choices in php (edit: by consumers of the UI framework), especially if php is the only place where we know the exact context where a component is being rendered. But maybe there is a goal that we can define and work towards?
Thx @catenglaender for your thoughts on this!
Just to clarify, we don't argue that PHP should not be used to make design decisions, we only say that developers using a component like this should not have to make them. This is kind of our philosophy, also described in our README file.
But shouldn't this be applicable using CSS container queries, to hide the label if the surrounding container exceeds a certain width? Or could we use something else for measure, and e.g. always hide labels for mobile?
I would like to implement withHiddenLabel()
only as a last resort.
Kind regards, @thibsy
Thanks @thibsy! I definitely like the idea of reacting to container space. This way neither the context has to be detected, nor the consumer using the UI component needs to set anything.
The range of things we should just automatically decide based on container space might be much larger than we are currently aware of. The question remaining would be in what cases we do allow loading of alternative design choices through a withThisAndThat()
.
- [ ] C\ViewControl\Sortation::withHiddenLabel(): this actually forces consumers to make a design decision, which is not something we want to do. I know this method has been introduced in order to amend for the functionality shown in the examples\ViewControls\Sortation\small example, but this should be achieved using (S)CSS only. Could you look into this? Maybe @catenglaender can help us here. If we can't make this happen, we should think about this functionality again.
@catenglaender, @klees and myself were thinking a little about the use and meaning of the label next to the ⇵-symbol, and I wanted to at least bounce our thoughts:
The \Input\ViewControl does not feature that label at all; that is because usually the order of the affected data is self-explaining and/or indicated by other means, like the arrow in header-cells. Displaying the order in the control, however, becomes necessary when there is no other and obvious way to distinguish the selected order. As a result - it's correct that withHiddenLabel is a decision about design and should not be put to the consumer - the decision about the necessity to display the label would require knowledge about the way the data is displayed and thus become a consumer choice again. So, we also might switch the defaults here: only render the ⇵-symbol in general, but give consumers the choice to force the label. That would further align \Input\Sortation and ViewControl\Sortation, too.
On the other hand, the \ViewControl\Sortation is mainly used in Panels and PresentationTables, where the label would usually be required, so opting-in the default would make no sense and I'd probably put the flag in the constructor.
Last, we could of course go with the proposed CSS solution.
What do you think?
- [x] Unit tests: the sortation view control HTML in the adjusted unit tests does not feature the components ID. Since the ID has been rendered before, could you please look into this? thx!
The id depends on the (Additional)OnLoadCode; this is only attached if Signals are being used with the ::withOnSort() function.
Hi @nhaagen,
Thx for sharing your thoughts, and implementing the requested changes.
@Amstutz, @klees and I have discussed the ordering label today, and came to the conclusion to address this issue with (S)CSS only. We therefore abandon the functionality to hide the ordering label "manually", and make this decision purely based on styling.
So to continue, please:
- [x] CSS: hide or show the ordering label according to the surrounding elements current width. This should be achievable using container queries.
- [x] Example: remove the
examples\ViewControls\Sortation\small
example again. Optionally, you could also showcase the CSS working properly there by rendering the component in a very small div. In that case you may keep the example in.
Thanks for bearing with us (again)!
Kind regards, @Amstutz, @klees and @thibsy
There is something off with the z-index of the dropdown menu as children of divs with container queries always have the z-index of the parent. I will look into that. Sadly, CSS container queries seem to have a lot of caveats to it. :disappointed:
Hello @alex40724 and @chfsx , we would be very glad if you could give us another review.
Best regards Oliver
Thx @oliversamoila for pushing here =)
Hello @alex40724 we would be very glad if you could give us your review.
Best regards Oliver