Skip to content
This repository has been archived by the owner on Sep 23, 2022. It is now read-only.

Better documentation #85

Open
smalot opened this issue May 14, 2013 · 15 comments
Open

Better documentation #85

smalot opened this issue May 14, 2013 · 15 comments

Comments

@smalot
Copy link
Contributor

smalot commented May 14, 2013

Hi all,

First of all, thanks for this great work. 馃憤
I've tested routing, forms and datatables and it's really easy to use.

With symfony 2.2 I've been obliged to use this :

class App extends Bundle
{
    public function boot()
    {
        Request::enableHttpMethodParameterOverride();
    }
}

Concerning the globale documentation, it is really poor.
For example, reading the code, I found how to add button in the table building this var :

public function indexAction()
{
    $items = $this->getRepository('App:User')->findAll();

    $routes = array(
        'show' => array(
            'path' => 'app_user_show',
            'label' => 'Show',
            'parameters' => array('id' => 'id')
        ),
        'edit' => array(
            'path' => 'app_user_edit',
            'label' => 'Edit',
            'parameters' => array('id' => 'id')
        ),
        'delete' => array(
            'path' => 'app_user_delete',
            'label' => 'Delete',
            'parameters' => array('id' => 'id')
        ),
    );

    return array('items' => $items, 'routes' => $routes);
}

and setting it in the twig template like that :

<table class="table table-hover table-bordered table-condensed">
    {{ bootstrap_datatable(items, {bootstrap: 'Twitter_Bootstrap', routes: routes}) }}
</table>

But I haven't been able to find such documentation about it.

Thanks for your feedback.

@docteurklein
Copy link
Contributor

Yes indeed the datatable is not very finished actually.
We would be more than happy if you would like to put these additions to the docs:

https://github.com/KnpLabs/rad-edition-site

Just make a pull request, and the changes would be automatically deployed after the merge.

Moreover, the wiki can accept modification / additions too: https://github.com/KnpLabs/KnpRadBundle/wiki/_pages

PS: concerning your code, I would prefer put this config in the twig template directly:

<table class="table table-hover table-bordered table-condensed">
    {{ bootstrap_datatable(items, {bootstrap: 'Twitter_Bootstrap', routes: { 
        show: { path: 'app_user_show' }}
    ) }}
</table>

@smalot
Copy link
Contributor Author

smalot commented May 15, 2013

I'm ok to append documentation to your wiki.
When do you plan to end datatable ? It's a very nice tool, and I would like to implement it in the backoffice of my futures projects.

What is "KnpLabs/rad-edition-site" ?

Concerning the config in my example, I think I'll put it in a yaml config file.
It will be clearer and it won't overload the twig template.

Thanks for all

@docteurklein
Copy link
Contributor

Great! :)

The rad-edition-site is the code behind http://rad.knplabs.com .
It's a simple jekyll app that contains our doc.

(PS: my example was for the docs).
Thanks!

@docteurklein
Copy link
Contributor

Concerning the changes, there are some internal renamings that needs to be done (like bootstrap option, see #86 )
and other stuff (like twig blocks than are badly named.

Moreover, the array of route options are too much. We should better generate directly the url using path().

@docteurklein
Copy link
Contributor

at the end it would look like:

<table class="table table-hover table-bordered table-condensed">
    {{ bootstrap_datatable(items, {theme: 'Twitter_Bootstrap', links: { 
        show:   { url: path('app_user_show', { id: user.id }) },
        delete: { url: path('app_user_delete', { id: user.id }) }
    ) }}
</table>

@smalot
Copy link
Contributor Author

smalot commented May 15, 2013

I think both implementations are interesting.
In the current implementation, you indicate only which property to use to build the path. This is very useful when your routes are used all over an app. So, thanks to a config file you can easily change the patterns :

first version of user path

/app/users/{id}

corresponding config file

routing.show:
    path: app_user_show
    label: Show user profile
    parameters:
        id: id

second version of the user path

/app/users/{username}

corresponding config file

routing.show:
    path: app_user_show
    label: Show user profile
    parameters:
        username: username

However, the standard implementation you are suggesting is effectively easier.
So, I think we should agree both of them.
When "url" is defined, we use it, otherwise we build the url from routes information as currently.

@smalot
Copy link
Contributor Author

smalot commented May 15, 2013

I note too that block names / routes, are not corresponding to routes auto created :

Tokens used by block are :

  • show
  • edit
  • delete

However, routes name are ending with :

  • view
  • edit
  • remove

Config file used to enable buttons in datatable :

    app_user:
        routes:
            show:
                path: app_user_view
                label: View user profile
                parameters:
                    id: id
            edit:
                path: app_user_edit
                label: Edit user profile
                parameters:
                    id: id
            delete:
                path: app_user_remove
                label: Remove user profile
                parameters:
                    id: id

It should be better to use the same naming.

@docteurklein
Copy link
Contributor

yes indeed!

@smalot
Copy link
Contributor Author

smalot commented May 15, 2013

oups, that was a mistake from my side.
forget my previous message

@smalot
Copy link
Contributor Author

smalot commented May 15, 2013

datatable headers are build like this :

<th id="header-lastname">lastname</th>

I think it's a good idea to autogenerate "id".
I suggest instead to generate a "class" attribute with the same value.
With "id", you can't have 2 table with a list of entity with for example an "firstname" attribute.

@docteurklein
Copy link
Contributor

sure! this is indeed not good.

@docteurklein
Copy link
Contributor

Moreover, we do not need an ID on this element. so why generating one? Maybe we even do not need classes.

@lsmith77
Copy link
Contributor

indeed .. seems there are a lot of undocumented features.

@gquemener
Copy link
Contributor

Not so many, I'd say 90% of the rad bundle is presented/documented either on rad.knplabs.com or on the wiki. But I agree on the fact that, in the end, it's not clear enough. That's why I'm +1 to reuse all this content to make a clear and up-to-date documentation as proposed here #90

@lsmith77
Copy link
Contributor

ah, I haven't looked at the wiki yet

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

No branches or pull requests

4 participants