Big addColumn, addField and Seed refactor (PART1) #179

romaninsh · 2017-06-19T10:52:45Z

Refactor to have a consistent addField / addColumn mechanics implementing #188 and #187 as well as incorporating atk4/core#50.

addColumn, addField and addColumnDecorator arguments should be consistent
new Seed should be respected
addColumn may not be called twice on same column
adding addDecorator (instead of Formatter) because formatting is done in UI Persistence
add consistent arguments (, <form field/table column> = null, = null)
significantly improve docs

Things to do:

create a good testSutie for all addColumn addDecorator and addField scenarios
reflect in the code
reflect in the docs

…mns-fields

…s-fields

…into feature/refactor-columns-fields

romaninsh · 2017-09-03T22:10:20Z

Updating to version 1.3 of Agile Core..

romaninsh · 2017-09-05T00:37:22Z

Given a huge scope of this commit, I propose that it would be split up into 3 stages:

First - this PR. It won't pass the tests, but the Form at least will work OK
Second PR will deal with addColumn()
Third PR will resolve all other problems with test suite.

It would be easier to review all PRs individually then merge them all simultaneously.

Also - the current 'develop' branch is broken because it's not compatible with core/develop.

…into feature/refactor-columns-fields

romaninsh · 2017-09-10T13:30:51Z

With the recent addition of documentation the Form should now be a first rate citizen of Agile UI.

I have used approach similar to basic elements where I start explaining it from the very basics and then gradually look at various use-cases while laying documentation against the classes, methods and properties.

romaninsh · 2017-09-10T14:20:46Z

src/FormLayout/Generic.php

        }

-        return $this->_add($field, ['name'=>$field->short_name]);
+        if (is_string($decorator)) {
+            $decorator = $this->form->decoratorFactory($field, ['caption'=>$decorator]);


I am myself unsure about this code. IIRC, Decorator should indicate type and third argument may contain caption.

romaninsh · 2017-09-10T14:52:57Z

src/Tabs.php

+        $sub->setAttr('data-tab', $item->name);
+
+        //# TODO: refactor this ugly hack
+        $item->setPath(str_replace('.php.', '.', $this->app->url($url)).'#');


@ibelar this is a nasty hack, can you see if that can be improved?

…ields

DarkSide666

there are some comments, but also some changes required.

DarkSide666 · 2017-09-13T17:15:11Z

demos/form5.php

@@ -0,0 +1,75 @@
+<?php


No link to this demo page in menu.

should be OK for now, i suppose we will be re-making the demo suite anyways.

DarkSide666 · 2017-09-13T17:30:13Z

demos/form5.php

+$f->addField('five', new \atk4\ui\FormField\Checkbox());
+
+// can add field that does not exist in a model
+$f->addField('nine', new \atk4\ui\FormField\Checkbox(['caption'=>'Caption3']));


This is really good option. Previously in toolkit I have had issues with this.

and i think it will change again, now it's new Checkbox('Caption 4')

DarkSide666 · 2017-09-13T17:33:34Z

docs/tablecolumn.rst

+will include "sorting" icons or any other controls that go in the header of the table.
+
+The output of this field will automatically encode any values (such as caption), shorten them
+if necessary and localize them.


confused by that text, so i deleted.

DarkSide666 · 2017-09-13T17:34:20Z

docs/tablecolumn.rst

+
+.. php:method:: getTotalsCellHTML(\atk4\data\Field $f, $value)
+
+Provided with the field and the value, format the cell for the footer "totals" column. Table


... footer "totals" row not column.

DarkSide666 · 2017-09-13T17:43:43Z

docs/type-presentation.rst

+show it without masking just for the admin? Change type in-line for the model field::
+
+    $m = new User($app->db);
+    $m->getElement('password')->type = 'string';


will password save normally in this case? will it still use encoding?

i don't think there is password encoding anywhere yet, but in this case, no , it won't use encodyngi.

added note.

DarkSide666 · 2017-09-13T18:31:45Z

src/Form.php

-                        return $this->success('Form data has been saved');
-                    } else {
-                        return new jsExpression('console.log([])', ['Form submission is not handled']);
+                try {


Do we really need 3 inherited try blocks?
We can use

try { ... } catch (\atk4\data\ValidationException $val) { ... } catch (\Error $e) { ... } catch (\Exception $e) { ... }

I leave this to you to refactor later, can you do it? I am not sure if php 5.6 supports multiple catch blocks, but 7.1 certainly good by adding https://wiki.php.net/rfc/multiple-catch.

I didn't mean multiple-catch like catch (\Error $1 || \Exception $e2) syntax. I mean not nesting try/catch blocks.

DarkSide666 · 2017-09-13T18:38:18Z

src/FormField/Input.php

@@ -40,6 +40,8 @@ class Input extends Generic

    public $actionLeft = null;

+    public $width = null;


description of possible options and data type ?

DarkSide666 · 2017-09-13T18:40:59Z

src/FormLayout/Generic.php

-        } elseif (is_array($args) && isset($args[0])) {
-            $args['caption'] = $args[0];
-            unset($args[0]);
+        if (!is_string($name)) {


it's already defined in function definition addField(string $name, ... so isit really necessary to check that here too?

yes, since user can call $layout->addField too.

DarkSide666 · 2017-09-13T18:44:19Z

src/FormLayout/Generic.php

+            $existingField->setDefaults($field);
+            $field = $existingField;
+        } elseif (is_object($field)) {
+            throw new Exception(['Duplicate field', 'name'=>$name]);


in what case $field can be object?

i feel this code still needs cleanup after latest changes to seed.

DarkSide666 · 2017-09-13T18:48:41Z

src/Grid.php

+     * an.
+     *
+     * @param string $name      New or existing column name
+     * @param [type] $columnDef [description]


incorrect @param

actually must be refactored to match Table

romaninsh and others added 4 commits June 18, 2017 14:25

Added comments.

a8f452a

a lot of refactoring for Aglie UI 1.2

27e3be4

Apply fixes from StyleCI

8d5f06c

Merge branch 'feature/add-form-validation' into feature/refactor-colu…

54999d7

…mns-fields

romaninsh mentioned this pull request Jun 19, 2017

Reimplemented Form Validation #177

Merged

8 tasks

romaninsh added the in progress label Jun 20, 2017

romaninsh self-assigned this Jun 20, 2017

romaninsh added this to the 1.2 - Next Major Branch milestone Jun 20, 2017

romaninsh and others added 7 commits June 25, 2017 22:46

Merge branch 'develop' into feature/refactor-columns-fields

324833d

work in progress

1bdaa6d

form testing file

8a52b73

Apply fixes from StyleCI

37f910c

Merge branch 'develop' into feature/refactor-columns-fields

1c9f469

Merge branch 'feature/misc-improvements' into feature/refactor-column…

7449eb2

…s-fields

still working on this

1ae13af

romaninsh added Consistency Documentation 📚 Undocumented features refactor labels Jul 9, 2017

Merge branch 'develop' into feature/refactor-columns-fields

e7ba7f9

romaninsh changed the title ~~Feature/refactor columns fields~~ Big addColumn, addField and Seed refactor Jul 28, 2017

romaninsh and others added 6 commits August 25, 2017 13:55

work in progress

bb68906

Apply fixes from StyleCI

b286a53

Merge branch 'develop' into feature/refactor-columns-fields

fcabc87

tabs.pug wouldn't compile.

67010de

Merge branch 'feature/refactor-columns-fields' of github.com:atk4/ui …

d07f94f

…into feature/refactor-columns-fields

Merge branch 'develop' into feature/refactor-columns-fields

92ab722

romaninsh and others added 3 commits September 3, 2017 23:42

Refactored addField() arguments for #187

df986d8

Apply fixes from StyleCI

1a03993

Furher implement addField() refactor

337e34b

Apply fixes from StyleCI

2191d04

romaninsh changed the title ~~Big addColumn, addField and Seed refactor~~ Big addColumn, addField and Seed refactor (PART1) Sep 4, 2017

romaninsh had a problem deploying to agile-ui-addfield September 5, 2017 13:54 Failure

romaninsh mentioned this pull request Sep 5, 2017

Return HTML from hooks to self-replace. #214

Closed

romaninsh and others added 7 commits September 6, 2017 13:47

Refactor form demo and resolve #187

6797bf9

rewriting form documentation

676c152

Merge branch 'feature/refactor-columns-fields' of github.com:atk4/ui …

ef6a600

…into feature/refactor-columns-fields

Apply fixes from StyleCI

3e2a2d7

added documentation

c191776

Merge branch 'feature/refactor-columns-fields' of github.com:atk4/ui …

16a3356

…into feature/refactor-columns-fields

Apply fixes from StyleCI

ad2e8d7

romaninsh commented Sep 10, 2017

View reviewed changes

romaninsh mentioned this pull request Sep 10, 2017

Big addColumn, addField and Seed refactor (PART2) #223

Merged

romaninsh requested a review from DarkSide666 September 10, 2017 23:15

romaninsh added Compatibility EPIC in review and removed in progress labels Sep 11, 2017

romaninsh changed the base branch from develop to epic/atk-core-refactor September 11, 2017 12:28

Merge branch 'epic/atk-core-refactor' into feature/refactor-columns-f…

b8b995e

…ields

DarkSide666 requested changes Sep 13, 2017

View reviewed changes

romaninsh and others added 4 commits September 14, 2017 12:45

Addressed Imant's change request

5e7a633

Apply fixes from StyleCI

b6f7675

I think this was used somewhere, forgot to add

0324dd4

Apply fixes from StyleCI

068a9f6

romaninsh merged commit 41cc057 into epic/atk-core-refactor Sep 14, 2017

DarkSide666 deleted the feature/refactor-columns-fields branch September 14, 2017 21:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Big addColumn, addField and Seed refactor (PART1) #179

Big addColumn, addField and Seed refactor (PART1) #179

romaninsh commented Jun 19, 2017 •

edited

Loading

romaninsh commented Sep 3, 2017

romaninsh commented Sep 5, 2017

romaninsh commented Sep 10, 2017

romaninsh Sep 10, 2017

romaninsh Sep 10, 2017

DarkSide666 left a comment

DarkSide666 Sep 13, 2017

romaninsh Sep 13, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 13, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 13, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 14, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

DarkSide666 Sep 13, 2017

romaninsh Sep 14, 2017

romaninsh Sep 14, 2017


		.. php:method:: getTotalsCellHTML(\atk4\data\Field $f, $value)

		Provided with the field and the value, format the cell for the footer "totals" column. Table

		@@ -40,6 +40,8 @@ class Input extends Generic

		public $actionLeft = null;

		public $width = null;

Big addColumn, addField and Seed refactor (PART1) #179

Big addColumn, addField and Seed refactor (PART1) #179

Conversation

romaninsh commented Jun 19, 2017 • edited Loading

romaninsh commented Sep 3, 2017

romaninsh commented Sep 5, 2017

romaninsh commented Sep 10, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DarkSide666 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

romaninsh commented Jun 19, 2017 •

edited

Loading