EasyCorp / EasyAdminBundle

EasyAdmin is a fast, beautiful and modern admin generator for Symfony applications.
MIT License
4.07k stars 1.02k forks source link

BooleanField suddenly reversed #6018

Open Yoran-Memo opened 12 months ago

Yoran-Memo commented 12 months ago

In the detail page of an entity within a CrudController, the booleanField is suddenly reversed since the update.

To Reproduce I updated easycorp/easyadmin-bundle from (v4.7.7 => v4.8.4)

Make an CrudController for an entity and create a booleanField (Active for example). Create the entity so its shown on the indexpage. Go to the detail page of the entity.

(OPTIONAL) Additional context image (2) image (1)

danielsippel commented 11 months ago

Happened with the changes from v4.7.7 => v4.8.0. Actually trying to identify the exact point in the code.

Yoran-Memo commented 11 months ago

It looks like the css of the field-boolean field-label and field-value is switched.

I personally rewrote the elements in CSS back to standard:

.ea-detail .field-group.field-boolean .field-label {
    flex: unset;
    min-width: 0;
    margin: 0 15px 0 0 !important;
    width: 130px !important;
    text-align: right !important;
}

.ea-detail .field-group.field-boolean {
    flex-direction: row !important;
}

.ea-detail .field-group.field-boolean .field-value {
    flex: 1 !important;
    min-width: 66% !important;
    text-align: left !important;
}
ServerExe commented 11 months ago

And I also realized that the help text tooltip isn't showing properly image

Other than that, you can see that there is no question-mark icon anymore if a field contains a help-text. Is this a bug or the new design? The label is only underlined.

Is there a possibility to set a help-text option to display the text fully underneath the field? As it is already on the edit-page.

juban commented 9 months ago

I can confirm that even with latest 4.8.12 version, boolean field display in details views is still wrong as mentioned above.

Nispeon commented 8 months ago

I came here thinking it was a bug as well, but it looks like it's actually intended: https://github.com/EasyCorp/EasyAdminBundle/pull/5977#issuecomment-1779880015

danielsippel commented 8 months ago

@Nispeon haaa, thanks for finding out :-)

danielsippel commented 8 months ago

Vote this comment up for: Leave it like it is right now, I like the new version.

easycorp/easyadmin-bundle (v4.9.3):

Bildschirmfoto 2024-03-07 um 17 36 00
danielsippel commented 8 months ago

Vote this comment up for: Please revert the change, labels for checkboxes should go back to the left.

easycorp/easyadmin-bundle (v4.7.7):

Bildschirmfoto 2024-03-07 um 17 34 41
danielsippel commented 7 months ago

@javiereguiluz @john-dufrene-dev Can we revert the flip of label/value? Or as an alternative add a setting in the Dashboard/Crud config classes?

juban commented 7 months ago

After reading #5977 I'm still confused about the motivation of that change. Why would the flipped label / value be more relevant for boolean fields?

danielsippel commented 7 months ago

@juban Same here, no idea.

ServerExe commented 7 months ago

After reading #5977 I'm still confused about the motivation of that change. Why would the flipped label / value be more relevant for boolean fields?

I think when reading Javier's comment https://github.com/EasyCorp/EasyAdminBundle/pull/5977#issuecomment-1779880015 it's getting clearer what's the motivation behind that. Thing is that the label column is very narrow in most cases (or even all cases) and it wouldn't make sense to give the "yes" or "no" values so much space while giving the label too little.

But I am still for having some opt-in possibility for this swap.

juban commented 7 months ago

After reading #5977 I'm still confused about the motivation of that change. Why would the flipped label / value be more relevant for boolean fields?

I think when reading Javier's comment #5977 (comment) it's getting clearer what's the motivation behind that. Thing is that the label column is very narrow in most cases (or even all cases) and it wouldn't make sense to give the "yes" or "no" values so much space while giving the label too little.

But I am still for having some opt-in possibility for this swap.

Well, as I understand the comment related to label length being possibility much larger than its value, what's still not clear is why it should apply more to a boolean value than any other field type. To present constantly anything one way and suddenly reverse the order for boolean is a bold move, to say the least. I know that the intention was good, but on an UX perspective, it's kind of a non sense to me.

Anyway, I also agree that it should be an opt-in option, and not the default behavior.

danielsippel commented 1 month ago

We switched it back with custom CSS:

.ea-detail .field-group.field-boolean {
    flex-direction: unset;
}
.ea-detail .field-group.field-boolean .field-label {
    flex: unset;
    margin: unset;
}
.ea-detail .field-group.field-boolean .field-value {
    flex: unset;
    width: 55px;
}

We have some more lines of custom CSS, didn't check if this is generally working for you guys.

Where this change came into action:

https://github.com/EasyCorp/EasyAdminBundle/commit/43cc8da92981dbe668275e9822c5cbb73489a225#diff-343fae5f1e255816a88ee3381904e289a34ed46240f2d4d72aa579844f38e210

thomas-l commented 1 month ago

The CSS revert from Yoran-Memo previously in this thread is working for me, with no other CSS involded in v4.12.0.