Download & Extend
Issues

tplify search module

Posted by dvessel on August 2, 2007 at 6:40pm
Project:Drupal core
Version:6.x-dev
Component:theme system
Category:task
Priority:critical
Assigned:Unassigned
Status:closed (fixed)

Issue Summary

These three modules that look like they could use some conversions. As far as modules go, I don't think anything else needs converting.

- search --Currently working
- tracker --Maybe, maybe not?
- aggregator --postponed?

Converted and commited:

- poll: http://drupal.org/node/162871 --part of a bigger patch
- user: http://drupal.org/node/161510
- profile: http://drupal.org/node/162134
- block: http://drupal.org/node/159936
- forum: http://drupal.org/node/158536
- comment: http://drupal.org/node/160589

Converted and still open:

- theme.inc: http://drupal.org/node/141914
- book: http://drupal.org/node/160200
- actions: http://drupal.org/node/159862

Other related patches

- maintenance tpl: http://drupal.org/node/141727 --core commiters, please take a look.
- Related to forum templates. Adds ID's to the forum templates: http://drupal.org/node/53905

I'll change this into an issue for search.module once I have a patch up.

Login or register to post comments

Comments

#1

Posted by dvessel on August 7, 2007 at 6:01pm
Title:tplify * placeholder» tplify search module
Status:active» needs review

Lots of changes..

Name changes:
- *search_page to *search_results
- - It wasn't actually a page. All it does is wrap the results. Form fields above it are not even part of this template.
- *search_item to *search_result
- - I think it makes more sense.

Removed '_keys' suffix for the form ID. They were already unique and it made the variable key for the .tpl longer then it needed to be.

Search forms broken up for each input. Makes it easier to replace a submit button with a graphic. This is done often.

Template suggestions provided based on type of search. "user" or "node". Class also added in the parent template based on type.

Small modification to node.module so $result['extra'] array that gets passed to template_preprocess_search_result() are all keyed. This makes it easier for themers to alter the $info_split array inside search-result.tpl.php. (search result meta information).

Removed a ruleset for search.css. ".search-form p" doesn't even exist.

For the theme functions to work, another patch must be applied due to a bug.

http://drupal.org/node/164351#comment-285126

Thanks for reviewing.

AttachmentSizeStatusTest resultOperations
tplify_search_1.patch9.09 KBIgnored: Check issue status.NoneNone

#2

Posted by dvessel on August 7, 2007 at 6:11pm
Priority:normal» critical

#3

Posted by dvessel on August 7, 2007 at 6:19pm

Small fix on template suggestion.

AttachmentSizeStatusTest resultOperations
tplify_search_1b.patch9.11 KBIgnored: Check issue status.NoneNone

#4

Posted by dvessel on August 7, 2007 at 6:25pm

Forgot the -N parameter. Here it is again with the .tpl files.

AttachmentSizeStatusTest resultOperations
tplify_search_1c.patch16.12 KBIgnored: Check issue status.NoneNone

#5

Posted by Ralf on August 8, 2007 at 8:55am
Status:needs review» needs work

Quick review:

CVS Id tags broken and missing blank line after cvs id tags

#6

Posted by dvessel on August 8, 2007 at 5:17pm
Status:needs work» needs review

Thanks for looking but the cvs id tags are not broke. They are new files. It'll automatically get converted once it's commited. And I believe the blank line isn't an issue since my other patches that were formed the same way got committed.

#7

Posted by Ralf on August 9, 2007 at 12:43am

The cvs id tag won't converted because the tag is $Id$ and not $Id.
The blank line is needed to make the files consistent.

Please take a look at http://drupal.org/node/164935. Thanks

#8

Posted by dvessel on August 9, 2007 at 1:40am

Okay, still a relative CVS noob.

AttachmentSizeStatusTest resultOperations
tplify_search_1d.patch15.96 KBIgnored: Check issue status.NoneNone

#9

Posted by dvessel on August 29, 2007 at 5:30pm

Rerolled for latest head.

AttachmentSizeStatusTest resultOperations
tplify_search_1e.patch15.98 KBIgnored: Check issue status.NoneNone

#10

Posted by dvessel on August 30, 2007 at 1:05am

And another update to keep up with head.

AttachmentSizeStatusTest resultOperations
tplify_search_1f.patch15.43 KBIgnored: Check issue status.NoneNone

#11

Posted by Wim Leers on August 30, 2007 at 9:55am

Patch works as advertised. I've tried every .tpl.php file and they all worked. I didn't test every available variable though.

I also went through the descriptions in each tpl.php file. Minor fixes:
- spelling ("existance" -> "existence", "dependant" -> "dependent")
- style (lines longer/shorter than 78 characters)

Updated patch attached.

AttachmentSizeStatusTest resultOperations
tplify_search_1g.patch8.89 KBIgnored: Check issue status.NoneNone

#12

Posted by Wim Leers on August 30, 2007 at 10:09am

The new files weren't included, my bad. Updated patch.

AttachmentSizeStatusTest resultOperations
tplify_search_1h.patch15.73 KBIgnored: Check issue status.NoneNone

#13

Posted by dvessel on September 6, 2007 at 5:21pm

Thanks Wim Leers, here it is again after the split patch..

I'd say this is ready, please before it breaks again... :)

AttachmentSizeStatusTest resultOperations
tplify_search_1g_0.patch16.64 KBIgnored: Check issue status.NoneNone

#14

Posted by dvessel on September 6, 2007 at 5:26pm

arg, left a stray drupal_rebuild_theme_registry()..

AttachmentSizeStatusTest resultOperations
tplify_search_1i.patch16.63 KBIgnored: Check issue status.NoneNone

#15

Posted by dmitrig01 on September 17, 2007 at 4:02am

why did you change node.module's node_invoke_nodeapi

#16

Posted by dvessel on September 17, 2007 at 6:22pm

It's because what I mentioned above:

Small modification to node.module so $result['extra'] array that gets passed to template_preprocess_search_result() are all keyed. This makes it easier for themers to alter the $info_split array inside search-result.tpl.php. (search result meta information).

#17

Posted by dmitrig01 on September 23, 2007 at 5:32pm

sorry

#18

Posted by dmitrig01 on October 6, 2007 at 1:10am

Bot, please review this. I'm hoping this works

#19

Posted by dmitrig01 on October 7, 2007 at 4:28am

maybe now - trying again

AttachmentSizeStatusTest resultOperations
tplify_search_2i.patch0 bytesIgnored: Check issue status.NoneNone

#20

Posted by Tobias Maier on October 7, 2007 at 11:57am
Status:needs review» needs work

you upload is 0 bytes!
upload again, please

#21

Posted by dmitrig01 on October 7, 2007 at 5:27pm
Status:needs work» needs review

oops

AttachmentSizeStatusTest resultOperations
tplify_search_1i_0.patch16.63 KBIgnored: Check issue status.NoneNone

#22

Posted by ChrisKennedy on October 20, 2007 at 11:36pm

Needed a re-roll but seems to work according to my testing.

AttachmentSizeStatusTest resultOperations
tplify_search.patch9.64 KBIgnored: Check issue status.NoneNone

#23

Posted by ChrisKennedy on October 20, 2007 at 11:39pm

Correct patch.

AttachmentSizeStatusTest resultOperations
tplify_search_0.patch16.48 KBIgnored: Check issue status.NoneNone

#24

Posted by Crell on October 29, 2007 at 9:23pm
Status:needs review» reviewed & tested by the community

The theme_search_block_form() function is vestigial and now never used, so I removed it. Everything else looks sane to me.

AttachmentSizeStatusTest resultOperations
tplify_search_2.patch16.49 KBIgnored: Check issue status.NoneNone

#25

Posted by Dries on October 31, 2007 at 6:07pm
Status:reviewed & tested by the community» fixed

Alright, this looks useful. Committed to CVS HEAD. Thanks.

#26

Posted by Anonymous on November 14, 2007 at 6:11pm
Status:fixed» closed (fixed)

Automatically closed -- issue fixed for two weeks with no activity.