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

List #422

Closed
wants to merge 2 commits into from
Closed

List #422

wants to merge 2 commits into from

Conversation

norv
Copy link
Contributor

@norv norv commented May 19, 2013

List class for discussion: I actually didn't make any changes (at all), only converted list to a class. For the clients, the same call is used, createList($param).
Client code can optionally also create a List object.

I'd submit it so that we see it, know it exists, and eventually work it out better internally or externally: for example, we can factor it with an internal structure (instance variables from those parameters), and we can create subclasses to override the buildList() method.

norv added 2 commits May 19, 2013 18:34
The implementation is a simple adaptation of the former function in List.subs.

Signed-off-by: Norv <a.w.norv@gmail.com>
Signed-off-by: Norv <a.w.norv@gmail.com>
@emanuele45
Copy link
Contributor

What I've always been interested in is give the ability to extend the lists.
At the moment there is a hook there, but its use is quite limited (in fact it can just be used to add/remove things in the surroundings of the list (i.e. above/below), because columns and rows still depend on the function retrieving the data from the database.
TBH I'm not sure how to extend (efficiently) this part.
Of course one option could be to hook the functions giving the possibility to extend the queries, though that's a "difficult" way (i.e. it requires passing pieces of queries around that may not be that good), another option would be to add another hook able to "inject" elements directly into the array here (but it implies some more processing and most likely yet another query per hook, that could be "annoying"). Well, in that last case could be "enough" to allow multiple $this->_listOptions['get_items']['function'], without having to rely on another hook (and include the processing into the class itself.

Are we aiming at something like that?
Did I miss any other obvious option?

Another thing I did in another branch (that I don't remember if I pushed or not, for sure I didn't PR it) was to split the template in a way that it was possible to render the "body" of the table and the "additional_rows" separately, allowing to re-use at least one of the two templates when needed (this because certain times may be useful to maintain all the flexibility of the original template, but changing "deeply" the body for example, and in the current situation the only solution is to duplicate everything.

P.S.
I know this is out of the scope of this PR, but while we are discussing my beloved createList... 👼

@Spuds Spuds mentioned this pull request Nov 2, 2013
@eurich eurich closed this Nov 2, 2013
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants