Skip to content

UI: 44282, add padding to .c-form in maincontrols-slate-content #9195

Merged
Amstutz merged 1 commit into
ILIAS-eLearning:release_10from
lukastocker:r10_44282
Mar 25, 2025
Merged

UI: 44282, add padding to .c-form in maincontrols-slate-content #9195
Amstutz merged 1 commit into
ILIAS-eLearning:release_10from
lukastocker:r10_44282

Conversation

@lukastocker

@lukastocker lukastocker commented Mar 13, 2025

Copy link
Copy Markdown
Contributor

https://mantis.ilias.de/view.php?id=44282

NOTE

It is correct that ui-component classes like ".c-form" or ".c-input" do not get any padding. The padding has to be configured in the parent div. This happens with this PR.

But have in mind that these changes will also add more padding to all legacy UI implementations in the editor slate.
It was decided with @yvseiler that we do not implement a special solution for that as the old legacy forms should get exchanged with the new UI elements in the future.

Update of note above:
It was discussed today (by the CSS-Squat-Meeting participants) that we will set padding specifically for .c-form in .il-slate-content, as .c-form in #copg-editor-slate-content.

ILIAS 11 will get another implementation.

@lukastocker
lukastocker requested a review from Amstutz March 13, 2025 10:40
@alex40724

Copy link
Copy Markdown
Member

@lukastocker Thanks for the PR. Unfortunately this adresses only the ILIAS page editor. However the issue is a general one handling forms in slates. In my opinion we should not add new editor specific rules, since they have been declared as legacy. Additionally each component or plugin using forms in slates would have to solve the issue again. If forms are being used in the main content or in modals, consuming components do not need to take care of padding, too. Best regards, Alex

@Amstutz

Amstutz commented Mar 15, 2025

Copy link
Copy Markdown
Contributor

Hi @lukastocker

Thx a lot for the PR. @alex40724 thx for the feedback and investing time and effort to make this component better! I understand the point you are making.

@catenglaender : Should we not simply promote the padding on forms one layer higher, to: .il-maincontrols-slate-content , as far as I see, this would remedy the point alex is making, while still keeping our rule concerning paddings on components such as forms intact.

kindly @Amstutz

@catenglaender

catenglaender commented Mar 17, 2025

Copy link
Copy Markdown
Contributor

Thank you for the PR, @lukastocker. Thank you for your input, @alex40724.

It's absolutely correct that this will lead to an uncomfortable phase of whack-a-mole to re-add paddings, but I am afraid we really do have to go all in with removing padding and margin around all UI components.

In legacy UI we have quite a few unfortunate constructs where a single visual spacing turns out to be a unpredictable combination of two paddings and a margin from different wrappers and containers when you have a closer look. It makes re-combining, mixing and skinning UI a never ending fight. This is why I really recommend delegating the handling of margins and paddings to always the same location: the parent legacy or UI component.

The solution presented in this PR is okayish, but an even better one would be 100% precise and explicit:

.il-maincontrols-slate-content {
    .c-form {
        padding: ...;
    }
}

What this allows is to mix components with padding and some without (e.g. a banner or a highlighted message spanning over the entire width of the slate), while being absolutely confident that the UI component doesn't already come with a padding or margin that could accidentally stack.

This concept is relatively new in ILIAS, and with the forms we are for the first time approaching the situation where we have to re-add a bunch of paddings. I think this will be worth it.

Edit: Using the general slate instead of the page editor slate is probably a good compromise between general and specific

…itor-slate-content while being part of the maincontrols-slate-content.

https://mantis.ilias.de/view.php?id=44282

This was discussed in the CSS-Squad Meeting today (25.03.2025).

A new implementation will be done for ILIAS 11.
@lukastocker lukastocker changed the title UI: 44282, add padding to the editor slate content div. UI: 44282, add padding to .c-form in maincontrols-slate-content Mar 25, 2025
@Amstutz

Amstutz commented Mar 25, 2025

Copy link
Copy Markdown
Contributor

we had a short discussion in the css squad on this issue. we believe the proposed solution to be good for ILIAS 10, in ILIAS 11 we might propose an even more generic one.

@Amstutz
Amstutz merged commit 019aa55 into ILIAS-eLearning:release_10 Mar 25, 2025
Amstutz pushed a commit that referenced this pull request Mar 25, 2025
…itor-slate-content while being part of the maincontrols-slate-content. (#9195)

https://mantis.ilias.de/view.php?id=44282

This was discussed in the CSS-Squad Meeting today (25.03.2025).

A new implementation will be done for ILIAS 11.

(cherry picked from commit 019aa55)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants