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

features/MultiLine Form Field #700

Merged
merged 20 commits into from
May 9, 2019
Merged

Conversation

ibelar
Copy link
Contributor

@ibelar ibelar commented May 3, 2019

MultiLine field

New form input in order to allow add/edit multiple record within a form.

Usage

$f = $app->add('Form');
$f->setModel($invoice, false);

$sub_layout = $f->layout->addSubLayout('Generic');
$sub_layout->setModel($invoice, ['name', 'client_id', 'address']);

$sub_layout = $f->layout->addSubLayout('Generic');

//Add multi line input to the layout.
$ml = $sub_layout->addField('ml', ['MultiLine']);
$ml->setModel($invoice, ['item','cat','qty','price', 'sub', 'tx', 'grd'], 'Items', 'invoice_id');

// add onchange handler and update grand total.
$ml->onChange(function($rows) use ($f) {
    $total = 0;
    foreach ($rows as $row => $cols) {
        foreach ($cols as $col) {
            $fieldName = key($col);
            if ($fieldName === 'grd') {
                $total = $total + substr($col[$fieldName], 4);
            }
        }
    }
 return $f->js(true, null, 'input[name="g_total"]')->val(number_format($total, 2));

}, ['qty', 'price']);

Screen Shot 2019-05-03 at 11 06 02 AM

Demo file

multiline.php

Feature

  • Adding row will use Model/Field default value if set.
  • Multiple row deletion;
  • Calculate Expression or Callback value from Field definition;
  • onChange callback for entire row record or on specify field.
  • Validate according to Field definition in model.
  • Validation is apply on every row when form is saved.

Screen Shot 2019-05-03 at 11 06 31 AM

@codecov
Copy link

codecov bot commented May 3, 2019

Codecov Report

Merging #700 into develop will decrease coverage by 29.21%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff               @@
##             develop     #700       +/-   ##
==============================================
- Coverage      72.55%   43.34%   -29.22%     
- Complexity      1933     2047      +114     
==============================================
  Files            105      106        +1     
  Lines           4559     4960      +401     
==============================================
- Hits            3308     2150     -1158     
- Misses          1251     2810     +1559
Impacted Files Coverage Δ Complexity Δ
src/FormField/MultiLine.php 0% <0%> (ø) 110 <110> (?)
src/AccordionSection.php 0% <0%> (-100%) 3% <0%> (ø)
src/HelloWorld.php 0% <0%> (-100%) 1% <0%> (ø)
src/Layout/Centered.php 0% <0%> (-100%) 1% <0%> (ø)
src/FormField/Radio.php 0% <0%> (-100%) 16% <0%> (ø)
src/FormField/Money.php 0% <0%> (-100%) 6% <0%> (ø)
src/Columns.php 0% <0%> (-100%) 10% <0%> (ø)
src/TableColumn/Password.php 0% <0%> (-100%) 1% <0%> (ø)
src/FormLayout/Section/Generic.php 0% <0%> (-100%) 1% <0%> (ø)
src/FormField/TextArea.php 0% <0%> (-100%) 5% <0%> (ø)
... and 72 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f31c667...1622264. Read the comment docs.

@codecov
Copy link

codecov bot commented May 3, 2019

Codecov Report

Merging #700 into develop will decrease coverage by 3.73%.
The diff coverage is 0%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop     #700      +/-   ##
=============================================
- Coverage      72.55%   68.82%   -3.74%     
- Complexity      1933     2043     +110     
=============================================
  Files            105      106       +1     
  Lines           4559     4811     +252     
=============================================
+ Hits            3308     3311       +3     
- Misses          1251     1500     +249
Impacted Files Coverage Δ Complexity Δ
src/FormField/MultiLine.php 0% <0%> (ø) 110 <110> (?)
src/TableColumn/Status.php 82.35% <0%> (+8.82%) 12% <0%> (ø) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update f31c667...bb4266e. Read the comment docs.

@romaninsh
Copy link
Member

romaninsh commented May 3, 2019

I'd like $rows to be a model linked with array persistence. Also - user should be able to change that with effect on the UI.

@ibelar are you sending ALL the rows back to the frontend or just changed ones ?

EDIT: let's leave that for the future.

Copy link
Member

@romaninsh romaninsh left a comment

Choose a reason for hiding this comment

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

very valuable. I do want to change many things, but that can be done at a later time.

if (!$field->read_only) {
$m[$fieldName] = $this->app->ui_persistence->typecastLoadField($field, $value);
}
} catch (\atk4\core\Exception $e) {
Copy link
Member

Choose a reason for hiding this comment

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

we should only catch and display validation errors (they have a dedicated class).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Was not sure how to check for post load validation error. Thought that typecastLoadField was supposed to do so. Let me know how this should work and I can change it.

*/
private function renderCallback()
{
$action = isset($_POST['action']) ? $_POST['action'] : null;
Copy link
Member

Choose a reason for hiding this comment

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

could this be using callback's trigger value?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The callback url is render only once for the component but the same callback url is used for two different action.

private function getValueForExpression($exprField, $fieldName, $model)
{
switch ($exprField->type) {
case 'money':
Copy link
Member

Choose a reason for hiding this comment

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

ok, FOR NOW, but would have to refactor.

$expr = $exprField->expr;
$matches = [];

preg_match_all('/\[[a-z0-9_]*\]|{[a-z0-9_]*}/i', $expr, $matches);
Copy link
Member

Choose a reason for hiding this comment

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

rather should rely on Model's callbacks.. OK for now. Or rather Persistence should do this.

romaninsh and others added 10 commits May 3, 2019 19:54
add options property for multiline table look
change field test for saving field value. now use isEditable()
return this when saving rows
- Add width setting using multiline key in ui for addField
- update comments
add form as onChange callback parameter.
@romaninsh
Copy link
Member

can't merge, CI fails.

remove method onChange and replace with onLineChange
@ibelar
Copy link
Contributor Author

ibelar commented May 7, 2019

@romaninsh - ci should passed now. Rename onChange method to onLineChange...

ibelar and others added 2 commits May 8, 2019 11:42
remove unnecessary dependency
ibelar added 2 commits May 8, 2019 11:54
update action name so it is specific to MultilLine
@romaninsh romaninsh merged commit d437ff7 into develop May 9, 2019
@romaninsh romaninsh deleted the feature/multiline-formfield branch May 9, 2019 18:03
@romaninsh
Copy link
Member

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants