Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CrudField JS library, for interacting with fields inside a CRUD form #4312

Merged
merged 86 commits into from
Jun 10, 2022

Conversation

tabacitu
Copy link
Member

@tabacitu tabacitu commented Apr 5, 2022

WHY

BEFORE - What was wrong? What was happening before this PR?

When you wanted to show/hide/toggle/change fields with JS, you had to write quite a bit of JS / jQuery code. For each field type... you'd have to think about the proper selector, try things out. Then when you manipulate it, you don't know where the info is stored (is it inside the main input/select/textarea? inside a hidden input? etc) and you don't know WHAT you can do to it (eg. checkbox and radio behave differently, because that's how the HTML spec works).

AFTER - What is happening after this PR?

Doing the most common things with JS in a form is a lot easier, because Backpack provides:

  • a common way to interact with fields, that looks A LOT like our fluent PHP syntax;
  • examples for the most common scenarios;

So.. no matter what field you're trying to select or to target, you can do crud.field('price') and you'll get back an object with the things you need most of times:

  • properties: name, wrapper, input, value
  • methods for events: onChange() - also aliased change()
  • methods that do stuff: hide(), show(), enable(), disable(), require(), unrequire(), check(), uncheck()

With this nifty little library, to show/hide one field depending on another, you'd now just need to:

Step 1. Create a script with whatever you want to do (eg. in public/assets/js/admin/forms/product.js):

 crud.field('visible').onChange(function(field) {
    if (field.value == 1) {
        crud.field('visible_where').show();
    } else {
        crud.field('visible_where').hide();
    }
 });

Step 2. Load that script inside the Create/Update form, by doing this in your setupCreateOperation() or setupUpdateOperation():

        Widget::add()->type('script')->content('public/assets/js/admin/forms/product.js');

Optionally, we could eliminate the second step, using a naming convention. Backpack can check if a JS file exists that follows the naming convention - if yes, it could load it automatically. Not sure it's worth doing, though...


This small library has been specifically to cover all scenarios identified as "most common" in here: #4158 (comment) - and it does. You'll see in the examples below... they're all covered. Here's a video of me showing them off too:

Chromium.-.Add.product.__.Backpack.Admin.Panel.-.5.April.2022.mp4

HOW

How did you achieve that, in technical terms?

Created a small JS library for it. Just like we have crud.table on the ListOperation, now you can work with crud.field() and crud.fields() inside the Create and Update operations.

Is it a breaking change?

Non-breaking. But the syntax and functionality might not be 100% final - that's up for debate.

How can we test the before & after?

Step 1. Use this branch:

composer require backpack/crud:"dev-crud-form-javascript-library as 5.9.99"

Step 2. Inside a CrudController (for example in ProductCrudController in the demo), add the following to your setupCreateOperation() or setupUpdateOperation() to add the fields for the 10 most common examples: https://gist.github.com/tabacitu/248dd59da9b33debc26cb7496f205bb5#file-productcrudcontroller-php

Step 3. Create that public/assets/js/product-form.js file, and place the associated JS code for those 10 examples inside it: https://gist.github.com/tabacitu/248dd59da9b33debc26cb7496f205bb5#file-product-form-js

@tabacitu
Copy link
Member Author

tabacitu commented Apr 6, 2022

TODOs to move this forward:

  • get feedback - ping @pxpm , @promatik , @iMokhles , @zachweix , @rroblik , @firecentaur , @ashek1412, @mamarmite , @fronbow , @soufiene-slimi , @LemarinelNet , @bedoz
  • fix the unrelated problem where widgets are loaded twice (why?!?!?!);
  • test on all field types;
  • try different/custom/complex scenarios, to see if our current syntax and API work ok for them, if it still helps out or they would need different parameters / methods or something;
  • triple-check property and method names; once we publish this... we can no longer change them;
  • triple-check closure parameters; once we publish... that's it;
  • decide whether to automatically load a JS file, using a naming convention; I've decided NOT to use a convention upon launch;
  • write documentation for it (including all 10 examples) - 5.1 - add docs about CrudField JS API docs#349;
  • (maybe, a little later on) create a PHP syntax, as a separate PR, that would automatically write the necessary JS for the most common scenarios, if a JSON is present on the field; I still think the PHP syntax we explained here is a good one, but as single-dimensional arrays:
CRUD::field([
    'name' => 'extra_cost_value',
    'type' => 'number',
    'showWhen' => [ 'agreed', 'isChecked', 'AND', 'start_date', '>=', 'end_date', 'AND', 'end_date', '<=', date('Y-m-d') ], 
    'hideWhen' => [ 'agreed', 'isUnchecked', 'OR', 'start_date', '<', 'end_date', 'OR', 'end_date', '>', date('Y-m-d') ], 
]);

@margarizaldi
Copy link

Can't wait for this feature release. Please do your magic soon.

@tabacitu
Copy link
Member Author

Thank you @margarizaldi ! @pxpm please take a look and give me your take on it. Let's move this forward.

tipsyelves-rainbow-magic-3o84U6421OOWegpQhq

@rroblik
Copy link

rroblik commented Apr 14, 2022

I think I know the answer but... as it is not related to "pro" and this is a basic feature can we hope a v4 backport?

Thanks

@tabacitu
Copy link
Member Author

I think I know the answer but... as it is not related to "pro" and this is a basic feature can we hope a v4 backport?

Nope... Backpack v4 will only receive security updates in the future, no extra features. See version support here. We must focus on making v5 the best we can.

@rroblik
Copy link

rroblik commented Apr 15, 2022

Final reason to make me leave BackPack adventure with 🫤 emoji !

@pxpm
Copy link
Contributor

pxpm commented Apr 18, 2022

Final reason to make me leave BackPack adventure with 🫤 emoji !

Hey @rroblik sorry to hear that. Like @tabacitu said, we cannot focus on v5 roadmap if we kept pushing stuff into v4, but let me just add that there is nothing preventing you from using this in your v4 project, the code is here, under "Files", so you can just copy it to your project and use it.

Please don't think I am arguing, I am 100% in favor of finding a better suite for my needs if the current one does not suffice them and I hope you find yours.

Cheers

Copy link
Contributor

@pxpm pxpm left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pxpm left a long comment here with a few bugs.
@tabacitu moved it to #4329 in order to keep this thread short, to not distract people from giving feedback on the idea and API syntax itself.

* too, by exposing the main components (name, wrapper, input).
*/
class CrudField {
constructor(name) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've just double-checked the selection logic and it still sounds good to me! Good job here @pxpm ! 👏👏👏

@promatik could you please also give it a look too? Maybe you spot red flags I missed, like you did with looking up through ancestors. Look at constructor() but also at inputWrapper and mainInput please.

Comment on lines 120 to 128
enable(value = true) {
this.$input.attr('disabled', !value && 'disabled');
this.$input.trigger(`backpack:field.${value ? 'enable' : 'disable'}`);
return this;
}

disable(value = true) {
return this.enable(!value);
}
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have disable() and enable(), but now I'm wondering... In most cases, isn't it readonly that people need, not disabled? What do you guys think?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure it's more used than disabled, but definitely it's widely used and we should probably support it.

@tabacitu
Copy link
Member Author

tabacitu commented Jun 9, 2022

Update: I've decided NOT to use a convention, to automatically load JS files from a path. Yes, it's one extra step, but:

  • that extra step will leave breadcrumbs, which I think is a very good thing for transparency; when you go into that CrudController, you'll see it loads a JS file, and be able to see what it does; otherwise, a new developer that comes into the project can just come into that CrudController and say "WTF" because they don't understand why it behaves in a different way than they expect;
  • that extra step is a super-simple one, a one-liner;
  • that extra step not only teaches you how to use the CrudField JS API, but also load any other JS or CSS into any other Backpack page;

For those reasons, I've decided that, for now, we won't be automatically loading any JS files onpage. We can change our minds upon feedback, of course. It's easy to add later. But for now... no, let's not do that.

Another update: We're SUPER FREAKING CLOSE to launching! 🎉🎉🎉 Everything's working properly, we have docs, we have... EVERYTHING. Running the final checks before we launch 🥳

@maverjk
Copy link

maverjk commented Jun 9, 2022

Update: I've decided NOT to use a convention, to automatically load JS files from a path. Yes, it's one extra step, but:

  • that extra step will leave breadcrumbs, which I think is a very good thing for transparency; when you go into that CrudController, you'll see it loads a JS file, and be able to see what it does; otherwise, a new developer that comes into the project can just come into that CrudController and say "WTF" because they don't understand why it behaves in a different way than they expect;
  • that extra step is a super-simple one, a one-liner;
  • that extra step not only teaches you how to use the CrudField JS API, but also load any other JS or CSS into any other Backpack page;

For those reasons, I've decided that, for now, we won't be automatically loading any JS files onpage. We can change our minds upon feedback, of course. It's easy to add later. But for now... no, let's not do that.

Another update: We're SUPER FREAKING CLOSE to launching! 🎉🎉🎉 Everything's working properly, we have docs, we have... EVERYTHING. Running the final checks before we launch 🥳

A really good news ;)
There will be also possibility to retrieve data from an AJAX request in some kind of way (as explicitly described it in my old post)?

@tabacitu
Copy link
Member Author

There will be also possibility to retrieve data from an AJAX request in some kind of way (as explicitly described it in my #4312 (comment))?

Of course. We don't make it any easier, but because of the simple architecture, because this is pure JS / jQuery that you write inside the closures or JS files... you can do AJAX calls, intercept AJAX calls, whatever you want 🎉

@tabacitu tabacitu merged commit 9e1de79 into v5 Jun 10, 2022
@tabacitu tabacitu deleted the crud-form-javascript-library branch June 10, 2022 07:28
@tabacitu
Copy link
Member Author

nbc-the-office-oh-my-god-its-happening-huJmPXfeir5JlpPAx0

@tabacitu tabacitu changed the title Add JS library for interacting with fields inside a CRUD form Add CrudField JS library, for interacting with fields inside a CRUD form Jun 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

6 participants