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

Make browse_sort_links default wrapper tags easier #409

Closed
patrickmj opened this issue Nov 15, 2012 · 3 comments
Closed

Make browse_sort_links default wrapper tags easier #409

patrickmj opened this issue Nov 15, 2012 · 3 comments
Assignees
Milestone

Comments

@patrickmj
Copy link
Contributor

Looks like the second param to browse_sort_links is an empty array, and the function splices in some defaults:

$defaults = array( 'link_tag' => 'li', 'list_tag' => 'ul', 'link_attr' => '', 'list_attr' => '' );

$sortlistWrappers = array_merge($defaults, $wrapperTags);

`

But in practice, it looks like these are more working defaults in admin side:

array('link_tag' => 'th scope="col"', 'list_tag' => '')

At least on admin side, maybe making those the defaults would be easier for devs?

I'm not sure of consequences on public side yet.

@jeremyboggs
Copy link
Contributor

Noticed that $defaults['link_attr']and $defaults['list_attr'] aren't used anywhere in the function, unless I'm overlooking it.

I think the attributes for these various tags could be added much easier using the existing, and quite handy, tag_attributes function used in other functions.

This would also be another great function to use a partial for outputting the HTML, one that would take the partial as an argument so you could the same function in multiple places but output the HTML differently.

@ghost ghost assigned kimisgold Jun 27, 2013
@jimsafley
Copy link
Member

ensure that all those parameters do something, and remove them if they don't

@zerocrates
Copy link
Member

Everything appears to be used, the $defaults are merged into $sortlistWrappers

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

No branches or pull requests

5 participants