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

href should be an attribute rather than a property #142

Closed
lukewestby opened this issue Jul 24, 2017 · 3 comments
Closed

href should be an attribute rather than a property #142

lukewestby opened this issue Jul 24, 2017 · 3 comments
Labels

Comments

@lukewestby
Copy link
Member

lukewestby commented Jul 24, 2017

When an a tag goes from having an href to having no href the patch leaves the attribute in place but with an empty string value instead of removing it. This is unexpected behavior, as a blank href has different semantics from no href at all. The cause of the behavior is that href is being set as a property, so a patch that removes it will set it to an empty string. Setting href as an attribute rather than a property should fix it.

Copying some context from https://github.com/elm-lang/virtual-dom/issues/109 below, including a link to an SSCCE

Problem

This bug is very easy to recreate and observe, I made a very basic ellie here

Long story short, here is what happens step-by-step:

  1. Start with an a element with no href, like a [] [], it can't be clicked.
  2. Toggle and switch to another a with an href like: a [ href "https://google.com" ] []
    • So far so good, we can now click the a and go to google.
  3. Toggle back to an a with no href, but now instead of having the original a [] [] (which is what we expect) we get the following html: <a href="">no href</a>. This is obviously problematic.

Workaround

  1. Use Html.Attributes.attribute "href" theHrefValue
@process-bot
Copy link

Thanks for the issue! Make sure it satisfies this checklist. My human colleagues will appreciate it!

Here is what to expect next, and if anyone wants to comment, keep these things in mind.

@lukewestby lukewestby added the bug label Jul 24, 2017
@amilner42
Copy link

Great workaround (and narrowing the problem) @lukewestby!

I can confirm it works for others who stumble into this issue.

@harfangk
Copy link

harfangk commented Jan 6, 2018

I just found out this issue too. I'm glad to see that it's already found and addressed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants