webiny-js icon indicating copy to clipboard operation
webiny-js copied to clipboard

Rules for Condition Groups

Open neatbyte-vnobis opened this issue 1 year ago • 9 comments

Changes

Added rules for condition groups.

How Has This Been Tested?

Manually.

Documentation

None.

neatbyte-vnobis avatar Dec 01 '23 15:12 neatbyte-vnobis

On this one, I've got two main points of feedback:

  1. UI: We've recently delivered a new feature called Advanced Search. I would like to use the look and feel of that accordion (styling, bg color) and the arrangement of the fields and conditions as well as the match all/any option. I think it will make the UI more uniformed across the apps.

  2. Submit & redirect: While talking to one of our customers about the conditional steps they brought forward a project they are currently working on where they need to send users to different pages depending on which step the user submitted. At the moment under Rules we have a dropdown with options: "Got to step" and "Submit", I would like to add here a 3rd option: Submit & Redirect. This would display an input where the user can define the url -> It should work the same way as the "Redirect" trigger that we have. The submit & redirect will take priority over the generic redirect trigger.

SvenAlHamad avatar Dec 07 '23 14:12 SvenAlHamad

And while I think we can tackle it, I think it's also important we check @SvenAlHamad's comments he posted some time ago.

Can we check that out? I'd guess tackling that feedback will introduce more changes into the PR.

I think that we can introduce requested changes in this PR.

neatbyte-ivelychko avatar Jan 03 '24 08:01 neatbyte-ivelychko

Hey @neatbyte-ivelychko @neatbyte-vnobis , I was just checking the lodash / @webiny/validation changes. Looks good... but...... I did some more thinking here.

I went to bundlephobia and entered @webiny/validation to see what's the size of @webiny/validation.

As you can see in the sshot, it's 28.4kb, which is not small.

image

But, the more interesting part is that the lib actually requires lodash internally 😅😅😅

All in all... I know we're going back and forth on this, but.... Since here we're talking about like.... what.... 6 validators? Can we just create a validators folder in page-builder-elements package and literally have validators manually coded in there? Without any libs?

Again, if this was an admin facing pkg, we wouldn't care. But we gotta care when it comes to code that's about to get served on public website. I believe that if we coded it manually like explained above, we'd probably end up with less then 1kb of extra code.

Wdyt?

adrians5j avatar Jan 04 '24 08:01 adrians5j

But, the more interesting part is that the lib actually requires lodash internally 😅😅😅

All in all... I know we're going back and forth on this, but.... Since here we're talking about like.... what.... 6 validators? Can we just create a validators folder in page-builder-elements package and literally have validators manually coded in there? Without any libs?

Again, if this was an admin facing pkg, we wouldn't care. But we gotta care when it comes to code that's about to get served on public website. I believe that if we coded it manually like explained above, we'd probably end up with less then 1kb of extra code.

Yeah, will do that as soon as I finish working on Submit & Redirect rule action

neatbyte-ivelychko avatar Jan 04 '24 09:01 neatbyte-ivelychko

But, the more interesting part is that the lib actually requires lodash internally 😅😅😅 All in all... I know we're going back and forth on this, but.... Since here we're talking about like.... what.... 6 validators? Can we just create a validators folder in page-builder-elements package and literally have validators manually coded in there? Without any libs? Again, if this was an admin facing pkg, we wouldn't care. But we gotta care when it comes to code that's about to get served on public website. I believe that if we coded it manually like explained above, we'd probably end up with less then 1kb of extra code.

Yeah, will do that as soon as I finish working on Submit & Redirect rule action

Should we do the same steps for app-form-builder or should we leave @webiny/validation there?

neatbyte-ivelychko avatar Jan 04 '24 09:01 neatbyte-ivelychko

Yes please.

Maybe it's worth exploring @webiny/app-form-builder-condition-groups-validation shared package? 🤔

Let's see how much code there will be and if there's a solid amount of code to c/p, then having a shared package might be a better idea.

adrians5j avatar Jan 04 '24 09:01 adrians5j

Yes please.

Maybe it's worth exploring @webiny/app-form-builder-condition-groups-validation shared package? 🤔

Let's see how much code there will be and if there's a solid amount of code to c/p, then having a shared package might be a better idea.

Btw. Maybe we should consider adding unit tests for validation as we have done it in IconPicker? Because we would not need to manually test validation methods.

neatbyte-ivelychko avatar Jan 04 '24 10:01 neatbyte-ivelychko

Btw. Maybe we should consider adding unit tests for validation as we have done it in IconPicker? Because we would not need to manually test validation methods.

@adrians5j what do you think about this?

neatbyte-ivelychko avatar Jan 10 '24 10:01 neatbyte-ivelychko

@SvenAlHamad Hi! I have updated UI for Rules tabs. Looking forward to your feedback!

1 - Rules for step Знімок екрана 2024-01-11 о 14 20 30 2 - Rules for condition group Знімок екрана 2024-01-11 о 14 22 30 3 - Rule with Submit & Redirect action Знімок екрана 2024-01-11 о 14 24 58

neatbyte-ivelychko avatar Jan 11 '24 12:01 neatbyte-ivelychko