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

Wrong boolean attributes conversion #119

Closed
stephannv opened this issue Mar 28, 2023 · 4 comments · Fixed by #120
Closed

Wrong boolean attributes conversion #119

stephannv opened this issue Mar 28, 2023 · 4 comments · Fixed by #120

Comments

@stephannv
Copy link
Contributor

ERB Input:

<input required>

Output:

input(required: "")

Expected output:

input(required: true)
@marcoroth
Copy link
Owner

marcoroth commented Mar 28, 2023

Thanks for reporting this @stephannv.

I don't think we can really support this, unless we hard code a list of attributes which should get forced to a boolean. Probably something like:

BOOLEAN_ATTRIBUTES = ["required", "disabled", "checked", "hidden", "readonly"]

But on the other hand, the output Phlexing is providing is not wrong, HTML boolean can either be written as:

<input required>
<input required="">

or:

<input required="required">

All of them are equally valid. MDN says:

If a boolean attribute is present, its value is true, and if it's absent, its value is false.
To be clear, the values "true" and "false" are not allowed on boolean attributes. To represent a false value, the attribute has to be omitted altogether.

So I'm not sure if this (and/or the boolean attribute mapping) should be handled here or in Phlex itself. Any thoughts?

@stephannv
Copy link
Contributor Author

I think it's not necessary to map all the possible boolean attributes, because some js frameworks can create new boolean attributes every day, eg. <v-input clearable>. I think this behavior should be simple: if <tag attribute> then tag(attribute: true), if <tag attribute=""> then tag(attribute: "").

I wrote some tests for this behavior:

  it "should convert boolean attributes properly" do
    html = %(<input required>)

    expected = <<~PHLEX.strip
      input(required: true)
    PHLEX

    assert_phlex_template expected, html
  end

  it "should convert blank attributes properly" do
    html = %(<input required="">)

    expected = <<~PHLEX.strip
      input(required: "")
    PHLEX

    assert_phlex_template expected, html
  end

But Nokogiri #value is always returning "" to <input required> and <input required="">, so I think this can be unfeasible due to Nokogiri behavior.

@marcoroth
Copy link
Owner

Thanks for providing some test-cases! I think that would make sense.

But Nokogiri #value is always returning "" to and , so I think this can be unfeasible due to Nokogiri behavior.

Yeah, I guess that's the root cause of the issue. I wonder if there's another method inside Nokogiri we might be able to use for that!

@stephannv
Copy link
Contributor Author

@marcoroth I found a solution, but I don't know if it's the best solution: #120

marcoroth pushed a commit that referenced this issue Mar 28, 2023
Nokogiri #value is always returning "" to <input required> and <input
required="">, so I have to call `#to_html` and check if the `=`
character is present. I'm not sure if this is the best way, but I tried
a lot of Nokogiri methods and none of them returns `nil`.

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

Successfully merging a pull request may close this issue.

2 participants