fomantic / Fomantic-UI

Fomantic-UI is the official community fork of Semantic-UI
https://fomantic-ui.com
MIT License
3.54k stars 329 forks source link

[checkbox/radio] Too much margin after 613 PR #2827

Open mvorisek opened 1 year ago

mvorisek commented 1 year ago

Bug Report

repro: https://ui.atk4.org/demos/form-control/form6.php

In atk4/ui we render field class for every form control. Such field can be anything, input, dropdown, group of checkboxes...

When there is group of checkboxes, the checkboxes have margin-top: 0.6em comming from https://github.com/fomantic/Fomantic-UI/blob/0540e9c291/src/definitions/collections/form.less#L194

The used HTML/CSS of "group of checkboxes" is based on docs https://fomantic-ui.com/modules/checkbox.html#radio

The styling was introduced in https://github.com/fomantic/Fomantic-UI/pull/613 and it seems it does not cover atk4/ui usecase well - https://github.com/fomantic/Fomantic-UI/pull/613#pullrequestreview-222319613 seems to be wrong.

Current style

image

Expected style (before the mentioned PR)

image

mvorisek commented 1 year ago

another repro: https://ui.atk4.org/demos/form-control/input2.php

there are two styles from the mentioned PR/613 matched, with one style disabled:

image

the result is even more wrong:

image

with whole PR reverted:

image

the styling is fixed.

ko2in commented 1 year ago

Can you please try with the below CSS in form.less and test all of use cases including yours?

.ui.ui.form .field .fields:not(.grouped) .field:not(:only-child) .ui.checkbox {
    margin-top: 0.6em;
}

If it does work, can you submit PR with the above fix?

I now have PR working in progress for some issues. It would be great if you submit PR for that.

mvorisek commented 1 year ago

@ko2in your CSS fixes https://github.com/fomantic/Fomantic-UI/issues/2827#issuecomment-1595505297 (when one style is disabled for debug)

but doesn't fix this issue (reproduced in https://ui.atk4.org/demos/form-control/form6.php nor https://ui.atk4.org/demos/form-control/input2.php demos), ie. too much vertical margin between radios

I belive the problem needs to be addressed in a different way - #613 reverted and with a new CSS style that will detect if there are other controls to which the height needs to be matched

or at least do not add any styling/spacing if grouped fields has only checkbox fields

ko2in commented 1 year ago

@mvorisek It fixes the first one.

radio-grouped-fields-bug

The second one doesn't work, because you've extra three fields wrapper.

To fix both, the following CSS would work. But I am not sure for overall use cases. It needs to test with every examples as described in documentation.

On line 193:

.ui.ui.form .field .fields:not(.grouped) > .field:not(:only-child) .ui.checkbox {
    margin-top: 0.6em;
}

On line 184:

.ui.form .fields:not(.grouped):not(.inline) > .field:not(:only-child) .ui.checkbox {
    margin-top: 2.41428571em;
}

If you've time, feel free to contribute.