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

Feature view img #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Feature view img #5

wants to merge 2 commits into from

Conversation

nypgand1
Copy link

@nypgand1 nypgand1 commented Feb 1, 2016

New element "ViewImg" for link with a img tag.

Used by issue #2 and template "Logo Nav" of Start Bootstrap.

http://startbootstrap.com/template-overviews/logo-nav/

@nypgand1
Copy link
Author

nypgand1 commented Feb 3, 2016

@mbr Hello there?

@mbr
Copy link
Owner

mbr commented Feb 10, 2016

Hey, I haven't forgotten this pull request and am looking into it.

@mbr
Copy link
Owner

mbr commented Feb 10, 2016

I'm willing to merge this for now, but cannot guarantee that it stays; but I'll try to make it work next time I'm hacking on Flask-Nav.

One of the issues I've had with this is that there are essentially two sets of **kwargs one would like to pass; the ones for url_for and any extra attributes for the img tag. Currently, img_src and img_alt are provided, however what if at some point you want an id or extra class on there? This could get ugly somewhat fast - especially if having an id will prevent you from using id as a parameter for url_for. It then becomes img_id and I feel it all might go downhill from there =).

An alternative I've been considering has been passing around dominate-tags, this would mean passing in the img-tag as an object. In this case, it would probably be more flexible to put this logic into View and renamed text to content; having everything work slightly similar to how it works with the JS DOM.

But, as I said above, I'll merge this, provided you drop the .gitignore =)

@nypgand1
Copy link
Author

Thanks for your advice and your great work!

@hstarmans
Copy link

hstarmans commented Apr 10, 2017

Hey @nypgand1 ,

Thanks for your code, I was also looking at how to implement a logo in my Navbar.
The code you made works, but produces two <li></li> tags.

<li>
      <li>
      <a href="/" title="logo">
        <img alt="logo" src="/static/logo.png">
      </a>
    </li>
    </li>

where i think it should be

   <li>
      <a href="/" title="logo">
        <img alt="logo" src="/static/logo.png">
      </a>
    </li>

The following should be sufficient:

def visit_ViewImg(self, node):
        a = tags.a(href=node.get_url(), title=node.text)
        a.add(tags.img(src=node.img_src, alt=node.img_alt))
        return a

@imakiro
Copy link

imakiro commented Oct 3, 2017

Hello,
I was wondering if the project was still alive and the feature present here could be merged.
Regards!

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

Successfully merging this pull request may close these issues.

4 participants