justintadlock / breadcrumb-trail

Official home of the Breadcrumb Trail plugin for WordPress.
GNU General Public License v2.0
136 stars 61 forks source link

Add false option to eliminate separator all together. #3

Closed kopepasah closed 9 years ago

kopepasah commented 10 years ago

Sometimes having a separator is not necessary and, as such, this new option will allow users to eliminate the separator.

I originally coded this using the new WordPress standard for braces, but other items of code in that section do not use braces for single line conditionals.

justintadlock commented 10 years ago

This is coming, but it will be slightly different. Essentially, I'd like for people to be able to do something like:

<ul>
    <li>Item</li>
    <li>Item</li>
</ul>

Or, just a flat list like normal but without a separator. I want to bring in both ideas at once though. So, I'd need to implement something like before_item and after_item parameters.

kopepasah commented 10 years ago

Yeah, I like that idea. Having the ability to completely customize the output would be cool.

However, if a user wants to maintain the current output and only get rid of the separator, would it not be easier to add the boolean false to the argument for the separator? Rather than having to create a completely new markup?

I do like your idea better, but I think having one option to remove the separator is easier for the user.

Is there a drawback to adding this that I am not seeing? Will it hinder the future development in regards to adding before_item and after_item parameters?

justintadlock commented 9 years ago

Closing this PR because the separator is no longer available in the latest code. Because it's a visual item, it should be handled with CSS.