Splitting off F from #1628044-33: Implement magic for the table of attachments on issues. As I wrote there:
However, instead of separate hook invocations, we could add another layer of nesting (keyed by 'visible' vs 'hidden') to the $rows and $headers arrays we pass to the alter hook. Hook implementations will have to know, but I think that's a reasonable API agreement to make in exchange for not doubling the # of hook invocations for this.
It makes things a bit more ugly for clients of this API, but it saves a lot of hook invocations. I think it's probably worth it, although I haven't looked closely enough at the new code (and all its various configuration options) to see how complicated this change would be.
Thoughts?
-Derek
| Comment | File | Size | Author |
|---|---|---|---|
| #2 | 1867102.patch | 11.4 KB | jthorson |
Comments
Comment #1
jthorson commentedThe solution here is to build out the render array (no matter whether it contains one table or two based on the settings), and pass the entire render array to the alter() hook before output.
Working on this along with #1867094: Pass more context to hook_file_metadata_table_items_alter().
Comment #2
jthorson commentedHere's what I came up with ... also cleaned up generation of the second table, to eliminate a ton of duplicate code.
Comment #3
jthorson commentedComment #4
jthorson commentedComment #5
jthorson commentedCommitted in 25ca17b.
Comment #6
dwwHaven't tested, but a brief skim of the code looks good. Thanks!
My only remaining concern would be renaming the alter hook to remove "rows" from the name. Perhaps
hook_file_metadata_table_elements_alter()would be more appropriate given these changes?Thanks again!
-Derek
Comment #7
jthorson commentedOne step ahead of you ... that happened in the next commit ... renamed to hook_file_metadata_table_output_alter().