-
Notifications
You must be signed in to change notification settings - Fork 902
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 fluent field group #2951
Add fluent field group #2951
Conversation
Just. Freaking. Awesome. Great work @pxpm ! This is the kind PR I love - surgical. Good job! Cheers! |
It's ok now @tabacitu you can test and merge. Best, |
@pxpm if using this code, removing
its working with nested group)
|
Indeed, calling the method directly allow us to remove the save call. Thanks |
But it works with column atm
|
Excellent idea with CRUD::group(
CRUD::column('name'),
CRUD::column('order')
)->type('text');
CRUD::group(
CRUD::field('name'),
CRUD::field('order')
)->type('text'); I think that's a great idea! |
What do you guys think of also supporting an array as parameter? (it's do the exact same thing) It looks to me like it's easy to get confused what the parameter(s) for the group method should be, so it might be a good idea to support both. CRUD::group(
CRUD::column('name'),
CRUD::column('order')
)->type('text');
// or
CRUD::group([
CRUD::column('name'),
CRUD::column('order'),
])->type('text'); Not sure about this one - asking for opinion. Personally I'd probably go with arrays in my projects because I like the fact that I can end all lines with a trailing comma. Though I believe it also works in parameters for PHP 7.3+ I believe... So that would render that argument mute 😆 |
I think you are right about the 7.3 trailling commas. But if you think we should support arrays for < 7.3 i am ok with it, but i personally would not be using the array notation myself. |
trailing comma works since php7.3, and i think, no need support array |
Added support for array input and refactored name to |
Hello @lotarbo This is pending review. We had some major bugs to work out first and freezed the introduction of features. I think it's stabilizing now and we should go back to features soon. Did you tried this PR? It's working good? Give us some feedback it might help speed up the review process. Best to you, |
hi, any progress in v6? |
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
[ci skip] [skip ci]
3181af1
to
5d42726
Compare
[ci skip] [skip ci]
[ci skip] [skip ci]
…ackpack/CRUD into add-fluent-field-group
[ci skip] [skip ci]
…ackpack/CRUD into add-fluent-field-group # Conflicts: # src/app/Library/CrudPanel/CrudField.php
REFS: #2642
Address #2950