atk4 / ui

Robust and easy to use PHP Framework for Web Apps
https://atk4-ui.readthedocs.io
MIT License
440 stars 104 forks source link

setControlsDisplayRules opposite behaviour #2129

Closed bedengler closed 7 months ago

bedengler commented 7 months ago

Strange behaviour of setControlsDisplayRules in v3.1

// This is a Lookup field
$adspaceSelection = $form->addControl('acpos_Adspace_ID', ['id' => 'adspaceSelection']);

$group1 = $form->addGroup();
// This is a Lookup field
$productSelection = $group1->addControl('acpos_Product_ID', ['id' => 'productSelection']);

$group2 = $form->addGroup();
// This is a Line field
$quantity = $group2->addControl('acpos_Quantity', ['id' => 'acpos_Quantity']);

$form->setControlsDisplayRules([
            'acpos_Quantity' => ['acpos_Product_ID' => 'not[empty]', 'acpos_Adspace_ID' => 'empty'],
        ]);

Expected behaviour: acpos_Quantity is shown, when acpos_Product_ID has a selection and acpos_Adspace_ID not. Real behaviour: exactly the other way round. So when acpos_Product_ID is empty and acpos_Adspace_ID has a selection, acpos_Quantity is shown.

When I do

$form->setControlsDisplayRules([
            'acpos_Quantity' => ['acpos_Product_ID' => 'empty', 'acpos_Adspace_ID' => 'not[empty]'],
        ]);

the result is the one I initially wanted, but it's against the logic...

Is the error in me, atk4 or FomanticUI? 🤪

mvorisek commented 7 months ago

In Fomantic-UI docs I found https://fomantic-ui.com/behaviors/form.html#/examples

So the meaning of empty is confusing and in reality, it should be named as notEmpty.

Is this how atk4/ui behave, anything to fix here?

To confirm, can you please test with is[dog] and not[is[dog]] to confirm?

mvorisek commented 7 months ago

kindly remainder @bedengler

bedengler commented 7 months ago

Where did you see the definition "empty is used there to "not pass an empty input""? According to the definition of FomanticUI, "empty" really means empty (which is logical). See here: https://fomantic-ui.com/behaviors/form.html#empty

Tested it with is[dog] etc. and that works as expected...

I also asked in the FomanticUI Discord now, if they know about that issue or even if this IS an issue 🤪 waiting for their reply...

mvorisek commented 7 months ago

Where did you see the definition "empty is used there to "not pass an empty input""? According to the definition of FomanticUI, "empty" really means empty (which is logical). See here: https://fomantic-ui.com/behaviors/form.html#empty

see the first code example in that link, and compare empty and checked there - empty is checking "if not empty" and checked "if checked"

bedengler commented 7 months ago

Alright, got confirmation from FomanticUI, it's the (wrong) behaviour since SemanticUI. So it's a "bug" they are carrying on and on. They pointed me to an article and look what I've found there: @mvorisek asking for the change from "empty" to "notEmpty" 🤪 https://github.com/fomantic/Fomantic-UI/discussions/2407#discussioncomment-3803102

It seems, they will not change that soon. Therefore I suggest 2 options: 1) transforming "empty" to "not[empty]" and "not[empty]" to "empty" on atk4 side. That way we are flexible and independent of FomanticUI OR 2) Adding a big notice about the behaviour in 1) the documentation, 2) the demos, 3) the source code of setControlsDisplayRules

What do you think should we do?

mvorisek commented 7 months ago

I have opened https://github.com/fomantic/Fomantic-UI/issues/2937 in Fomantic-UI. We should not try to fix every issue in atk4/ui, but instead support Fomantic-UI as much as possible.

mvorisek commented 7 months ago

Reopening as after it will be fixed by Fomantic UI, we can rename the empty usages in atk4/ui.

mvorisek commented 7 months ago

Since v1.5 - https://github.com/atk4/ui/blob/2.4.0/js/src/services/form.service.js#L40 - the notEmpty was already provided by atk4/ui, now it is provided officially.