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

Empty id and class atrributes #93

Closed
revdan opened this issue Mar 24, 2012 · 6 comments
Closed

Empty id and class atrributes #93

revdan opened this issue Mar 24, 2012 · 6 comments

Comments

@revdan
Copy link

revdan commented Mar 24, 2012

I'm using sinatra-simple-navigation 3.5.1 and if I don't specify an id or class in the navigation.rb config file, it's generating <ul id=' ' class=' '>

I've checked the documentation but can't find anything - am I doing something wrong?

@mjtko
Copy link
Contributor

mjtko commented Mar 24, 2012

Hi @revdan,

The Sinatra adapter is not currently intelligent enough to suppress rendering of attributes with nil values.

This could be fixed in two ways

  1. have the list renderer not specify the :id and :class keys if there are no values defined for them.
  2. add intelligence to the #to_attributes method in the sinatra adapter to only render attributes which have non-nil values.

Of course, this should have no effect on your application anyway, and I would consider this to be a purely cosmetic change.

Cheers,

Mark.

@revdan
Copy link
Author

revdan commented Mar 24, 2012

Hi Mark,

Thanks for the swift response. It was setting off my OCD ;)

Thanks,
Dan

@revdan revdan closed this as completed Mar 24, 2012
@mjtko mjtko reopened this Mar 24, 2012
@mjtko
Copy link
Contributor

mjtko commented Mar 24, 2012

Ah, that is something I can certainly appreciate! :-)

I'm going to reopen this and will spend a few minutes fixing it in master a little later today. Will be in the next release!

Cheers,

Mark.

@revdan
Copy link
Author

revdan commented Mar 24, 2012

Cheers Mark :)

I noticed this isn't happening on the demo page - is this specific to the Sinatra version?

@mjtko
Copy link
Contributor

mjtko commented Mar 24, 2012

Yes, it is specific to the Sinatra adapter - the Rails adapter delegates all content_tag work to the underlying framework. Sinatra doesn't contain as much magic as Rails (which is, of course, the point of Sinatra!) so, the Sinatra adapter contains a trivial content_tag implementation.

mjtko added a commit that referenced this issue Mar 24, 2012
…butes with empty strings in the output, instead \

electing not to render the attribute at all. Fixes issue #93. Thanks to @revdan for pointing this out.
@mjtko
Copy link
Contributor

mjtko commented Mar 24, 2012

Ok, fixed in c956410, will be out when I get round to making a 3.8.0 release - hope your OCD can wait until then! :-)

Cheers,

Mark.

@mjtko mjtko closed this as completed Mar 24, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants