Fix #629, also add new Callback for custom HTML #643

PhilippGrashoff · 2019-02-01T08:22:32Z

Adds property renderRowFunction, which can be used to pass custom html options to renderView for each option.
If not defined, only needed title_field and id_field are used. (and system fields, they are also loaded when using only_fields()).

Adds property renderRowFunction, which can be used to pass custom html options to renderView for each option. If not defined, only needed title_field and id_field are used

PhilippGrashoff · 2019-02-01T08:25:05Z

Some thoughts about this:

I only tested locally,. Need to read more into atk tests to write a test which complies with other tests sensibly.
could make sense to also use this function when values are set instead of model, what do you think?

codecov · 2019-02-01T08:27:18Z

Codecov Report

Merging #643 into develop will decrease coverage by 0.01%.
The diff coverage is 77.77%.

@@             Coverage Diff              @@
##             develop    #643      +/-   ##
============================================
- Coverage      69.81%   69.8%   -0.02%     
  Complexity      1853    1853              
============================================
  Files            100     100              
  Lines           4377    4382       +5     
============================================
+ Hits            3056    3059       +3     
- Misses          1321    1323       +2

Impacted Files	Coverage Δ	Complexity Δ
src/FormField/DropDown.php	`86.66% <77.77%> (-2.43%)`	`30 <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 d466e9e...0df3dea. Read the comment docs.

codecov · 2019-02-01T08:27:18Z

Codecov Report

Merging #643 into develop will decrease coverage by 0.03%.
The diff coverage is 92.68%.

@@              Coverage Diff              @@
##             develop     #643      +/-   ##
=============================================
- Coverage      66.05%   66.02%   -0.04%     
- Complexity      2223     2233      +10     
=============================================
  Files            118      118              
  Lines           5235     5262      +27     
=============================================
+ Hits            3458     3474      +16     
- Misses          1777     1788      +11

Impacted Files	Coverage Δ	Complexity Δ
src/FormField/DropDown.php	`88.09% <92.68%> (-3.14%)`	`41 <15> (+10)`
src/App.php	`69.84% <0%> (-2.39%)`	`117% <0%> (ø)`
src/Card.php	`62.61% <0%> (ø)`	`51% <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 6565b7c...8d5af3b. Read the comment docs.

DarkSide666 · 2019-02-01T11:27:29Z

src/FormField/DropDown.php

+            //record to callback
+            if (is_callable($this->renderRowFunction)) {
+                foreach ($this->model as $key => $row) {
+                    $options[] = call_user_func($this->renderRowFunction, $row);


could use

array_merge( ['div', 'class' => 'item', 'data-value' => (string) $key, [$row->getTitle()]], call_user_func(...) )

here so you don't have to always set all parameters in callback - just overwrite the ones you need.

Also it would be good to add same callback function support below if model is not set ($this->values is).
In this line https://github.com/atk4/ui/pull/643/files#diff-4b67e9492f67665780c2571ad53ebb68R225

PhilippGrashoff · 2019-02-01T17:22:06Z

Both things sound very sensible. Just don't get how to change files in my pull request... You gotta show me someday how to sensibly create atk4 pr s

romaninsh · 2019-02-05T23:04:19Z

Missing documentation and examples..

romaninsh

need documentation and examples..

otherwise i like the idea.

DarkSide666 · 2019-02-08T21:45:30Z

@PhilippGrashoff if you're mostly Windows guy like me or Mac and don't like to type every single command from command line then you can use free SmartGit program. It works very well. I use it for a few years - makes all git stuff a lot simpler and with nice UI.

P.S. I can't add changes in branch in your github account, so you'll have to make them yourself.
Otherwise I can just copy your changes and make new branch myself and tweak it.

PhilippGrashoff · 2019-02-09T11:43:44Z

@DarkSide666 maybe have a short Skype conversation sometime. I think my whole approach to creating PRs for atk is crap :)

p.s: I'm a Linux guy and using Gitkraken.

gowrav-vishwakarma · 2019-02-12T10:59:07Z

Gitkraken

If this is the case, I am also in line. One 30 minutes session can help, a bunch of bad "pull requesters" like me in action ;)

romaninsh · 2019-02-18T23:22:13Z

How about we have a weekly on-line session where we discuss all hot issues and also help contributors? I would certainly attend, but I'd prefer someone else to do the admin stuff (announce, etc)

PhilippGrashoff · 2019-02-19T09:33:36Z

Sounds good. I'd just need some push in the right direction for creating prs from git software. At the moment I edit the file on github.com by copy pasting the changes :) didn't get any atk4 pr to work in another way yet

abbadon1334 · 2019-02-20T12:49:20Z

i'm the king of PR messing :D in the last week i start thinking about start streaming on twitch out of working time while i'm coding/debug things just to do something and see what would happen.

I think a project like atk4 need more help to grow and make it better, we started an app for a client 3 weeks ago and we are near releasing alpha and considering time for debugging some atk4 things, it's unreal

gowrav-vishwakarma · 2019-02-21T04:56:10Z

Agreed on that, A short pull request system explained will help other willing contributors in action. [image: Mailtrack] <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality5&> Sender notified by Mailtrack <https://mailtrack.io?utm_source=gmail&utm_medium=signature&utm_campaign=signaturevirality5&> 02/21/19, 10:24:31 AM

…

On Wed, Feb 20, 2019 at 6:19 PM Francesco Danti ***@***.***> wrote: i'm the king of PR messing :D in the last week i start thinking about start streaming on twitch out of working time while i'm coding/debug things just to do something and see what would happen. I think a project like atk4 need more help to grow and make it better, we started an app for a client 3 weeks ago and we are near releasing alpha and considering time for debugging some atk4 things, it's unreal — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#643 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AA9hU2yMuihx9Ev4yRc9VAN1bnsMSrryks5vPURQgaJpZM4ad23H> .

-- -- Regards *Gowrav Vishwakarma* Founder & Managing Director *Xavoc Technocrats Pvt. Ltd.* http://www.xavoc.com contact: +91 961-0005-727, 723-0005-787

romaninsh · 2019-03-20T19:40:40Z

note: inconsistent with https://github.com/atk4/ui/blob/develop/src/Table.php#L504.

Then again - i think that dropdown does not implement hook trait.

PhilippGrashoff · 2019-05-05T12:29:08Z

Hi,

the idea with array_merge is nice, but as the array is only partly indexed, it wont work, at least not in a sensible way.
In general I am not happy with the solution I added. Would you mind if I split up renderView() a bit, introducing a method which is called _renderLines() which takes care of rendering the Dropdown elements?
That would make renderView() smaller and Dropdown would be easier to extend, e.g. if someone wants to just change the way the dropdown options are displayed (exceeding what a callback could do?)?

Best regards

romaninsh · 2019-05-07T22:14:01Z

@PhilippGrashoff if you are willing to improve your PR, - sure by all means, close this one and create a new on!

And thanks again for contributing.

- use Subtemplate for row rendering instead of $app->getTag(). Makes customization easier if needed (e.g. display complex HTML per row) - implement renderRowFunction which can be used to easily customize dropdown row rendering

PhilippGrashoff · 2019-05-21T20:12:40Z

HI there,

I refactored renderView quite a bit and used a subtemplate for row's HTML instead of $app->getTag(). Why?

Makes introduction of complex HTML easier, just set defaultTemplate to your own (thinking about this, maybe make $_tItem public to have the possibility to just overwrite this?)
move HTML from PHP file to Template file

Now the renderRowFunction works nicely IMHO, have a look at this:

        $this->mainForm->fields['tour_type_id']->renderRowFunction = function($row) {
            return [
                'value' => $row->id,
                'title' => strtoupper($row->getTitle()).' ('.$row->get('activity_id').')',
                'icon'  => 'calendar',
            ];
        };

   $this->mainForm->addField('test', [
            'DropDown',
            'values' => [
                1 => 'BLa',
                2 => 'dfdfd',
            ],
            'renderRowFunction' => function($value, $key) {
                 return [
                    'value' => $key,
                    'title' => strtoupper($value),
                    'icon'  => strpos('BLa', $value) !== false ? 'calendar' : 'money',
                ];
            },
        ]);

I didnt write any tests or documentation yet as I am not sure if what I did is the ATK way. I like it like this. Please give me some feedback and if you think this is fine I will provide documentation.

romaninsh · 2019-05-21T20:20:03Z

Sorry for not mentioning it right away - some examples in demo/ would be also nice.

Here is a spot for docs: https://github.com/atk4/ui/blob/develop/docs/field.rst#dropdown

(although it's really sparse there)

Finally - didn't we switch from "DropDown" to "Lookup" as a default dropdown implementation?

PhilippGrashoff · 2019-05-21T20:21:35Z

HI Romans,
yes, for hasOne() relations the standard UI has be switched to Lookup. But I guess 'normal' DropDown' will continue to be there? :)

romaninsh · 2019-05-21T20:22:41Z

@PhilippGrashoff actually, me and @ibelar made Lookup to be a copy of Dropdown then enhanced it.

The idea was to remove DropDown but then we forgot 😢

romaninsh · 2019-05-21T20:23:47Z

#637
#551

romaninsh · 2019-05-21T20:25:00Z

We can probably move one of those two into an add-on.

PhilippGrashoff · 2019-05-21T20:25:33Z

After a quick read of Lookup code:

seems like a new version of AutoComplete, always working with a Query to the server, right?
seems to only work with a model, not with values property as Dropdown?

To me it looks more as if Lookup was to replace AutoComplete, not Dropdown.

PhilippGrashoff · 2019-05-21T20:31:17Z

+1 for keeping dropdown, as

it renders all items right away without an additional request to the server
custom values can be defined using $values properties

ibelar · 2019-05-22T02:09:12Z

@PhilippGrashoff is right. New Lookup was for replacing Autocomplete. We should keep Dropdown.

romaninsh · 2019-05-23T18:04:37Z

Sorry, i was confused, we're obsoleting AutoComplete, not Dropdown.

romaninsh

good!

gowrav-vishwakarma

Looks okay, just need documentation and demos

PhilippGrashoff · 2019-05-26T11:20:27Z

Added Docs, but am too stupid to get the markup right. Will read into it.

dropdown-plus.php existed, but was not linked in demo menu

PhilippGrashoff · 2019-05-26T11:40:07Z

Hi, I also added Demos, but didnt test them yet as I am not sure how to do this best. Also, the formatting of the docs aint correct in some places.

DarkSide666

Tests fail:
PHP Fatal error: Cannot use empty array elements in arrays in /home/travis/build/atk4/ui/demos/dropdown-plus.php on line 35

Fix #629, also add new Callback for custom HTML

0df3dea

Adds property renderRowFunction, which can be used to pass custom html options to renderView for each option. If not defined, only needed title_field and id_field are used

DarkSide666 requested a review from romaninsh February 1, 2019 11:20

DarkSide666 added the in review label Feb 1, 2019

DarkSide666 requested a review from ibelar February 1, 2019 11:21

DarkSide666 reviewed Feb 1, 2019

View reviewed changes

romaninsh requested changes Feb 5, 2019

View reviewed changes

romaninsh added in progress and removed in review labels May 7, 2019

PhilippGrashoff added 2 commits May 21, 2019 22:04

Refactor renderView()

e624ae0

- use Subtemplate for row rendering instead of $app->getTag(). Makes customization easier if needed (e.g. display complex HTML per row) - implement renderRowFunction which can be used to easily customize dropdown row rendering

Implement use of Subtemplate for DropDown Rows

dda4804

romaninsh added the Documentation 📚 Undocumented features label May 21, 2019

romaninsh added the hangout agenda 🔈 Will discuss on next hangout label May 21, 2019

romaninsh requested a review from gowrav-vishwakarma May 23, 2019 18:05

romaninsh approved these changes May 23, 2019

View reviewed changes

romaninsh removed the hangout agenda 🔈 Will discuss on next hangout label May 23, 2019

gowrav-vishwakarma approved these changes May 24, 2019

View reviewed changes

Add DropDown Documentation

dda9d47

PhilippGrashoff added 2 commits May 26, 2019 13:35

Add DropDown Demos

3c9e954

Link Dropdown demos in Menu

7d13207

dropdown-plus.php existed, but was not linked in demo menu

DarkSide666 self-requested a review May 27, 2019 16:58

DarkSide666 requested changes May 27, 2019

View reviewed changes

PhilippGrashoff added 2 commits June 18, 2019 13:31

fix double ,

3f02a17

fix double form field name

8d5af3b

romaninsh approved these changes Jun 21, 2019

View reviewed changes

romaninsh merged commit ec078d9 into atk4:develop Jun 21, 2019

PhilippGrashoff mentioned this pull request Jul 2, 2019

#629 and #643: revert only_fields setting? #764

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix #629, also add new Callback for custom HTML #643

Fix #629, also add new Callback for custom HTML #643

PhilippGrashoff commented Feb 1, 2019

PhilippGrashoff commented Feb 1, 2019

codecov bot commented Feb 1, 2019

codecov bot commented Feb 1, 2019 •

edited

DarkSide666 Feb 1, 2019

DarkSide666 Feb 1, 2019

PhilippGrashoff commented Feb 1, 2019

romaninsh commented Feb 5, 2019

romaninsh left a comment

DarkSide666 commented Feb 8, 2019 •

edited

PhilippGrashoff commented Feb 9, 2019 •

edited

gowrav-vishwakarma commented Feb 12, 2019

romaninsh commented Feb 18, 2019

PhilippGrashoff commented Feb 19, 2019

abbadon1334 commented Feb 20, 2019

gowrav-vishwakarma commented Feb 21, 2019 via email

romaninsh commented Mar 20, 2019 •

edited

PhilippGrashoff commented May 5, 2019

romaninsh commented May 7, 2019

PhilippGrashoff commented May 21, 2019

romaninsh commented May 21, 2019

PhilippGrashoff commented May 21, 2019

romaninsh commented May 21, 2019

romaninsh commented May 21, 2019

romaninsh commented May 21, 2019

PhilippGrashoff commented May 21, 2019

PhilippGrashoff commented May 21, 2019

ibelar commented May 22, 2019

romaninsh commented May 23, 2019

romaninsh left a comment

gowrav-vishwakarma left a comment

PhilippGrashoff commented May 26, 2019

PhilippGrashoff commented May 26, 2019

DarkSide666 left a comment

Fix #629, also add new Callback for custom HTML #643

Fix #629, also add new Callback for custom HTML #643

Conversation

PhilippGrashoff commented Feb 1, 2019

PhilippGrashoff commented Feb 1, 2019

codecov bot commented Feb 1, 2019

Codecov Report

codecov bot commented Feb 1, 2019 • edited

Codecov Report

DarkSide666 Feb 1, 2019

Choose a reason for hiding this comment

DarkSide666 Feb 1, 2019

Choose a reason for hiding this comment

PhilippGrashoff commented Feb 1, 2019

romaninsh commented Feb 5, 2019

romaninsh left a comment

Choose a reason for hiding this comment

DarkSide666 commented Feb 8, 2019 • edited

PhilippGrashoff commented Feb 9, 2019 • edited

gowrav-vishwakarma commented Feb 12, 2019

romaninsh commented Feb 18, 2019

PhilippGrashoff commented Feb 19, 2019

abbadon1334 commented Feb 20, 2019

gowrav-vishwakarma commented Feb 21, 2019 via email

romaninsh commented Mar 20, 2019 • edited

PhilippGrashoff commented May 5, 2019

romaninsh commented May 7, 2019

PhilippGrashoff commented May 21, 2019

romaninsh commented May 21, 2019

PhilippGrashoff commented May 21, 2019

romaninsh commented May 21, 2019

romaninsh commented May 21, 2019

romaninsh commented May 21, 2019

PhilippGrashoff commented May 21, 2019

PhilippGrashoff commented May 21, 2019

ibelar commented May 22, 2019

romaninsh commented May 23, 2019

romaninsh left a comment

Choose a reason for hiding this comment

gowrav-vishwakarma left a comment

Choose a reason for hiding this comment

PhilippGrashoff commented May 26, 2019

PhilippGrashoff commented May 26, 2019

DarkSide666 left a comment

Choose a reason for hiding this comment

codecov bot commented Feb 1, 2019 •

edited

DarkSide666 commented Feb 8, 2019 •

edited

PhilippGrashoff commented Feb 9, 2019 •

edited

romaninsh commented Mar 20, 2019 •

edited