bikeshaving / crank

The Just JavaScript Framework
https://crank.js.org
MIT License
2.7k stars 75 forks source link

The type="text" attribute on input elements is being removed #257

Closed waynebaylor closed 1 year ago

waynebaylor commented 1 year ago

Crank version: 0.5.3

If you render the following:

import {renderer} from "@b9g/crank/dom";

function Form() {
  return (<form>
    <input type="text" />
    <input type="password" />
    <input type="email" />
  </form>);
}

renderer.render(<Form />, document.body);

The resulting html seen in the browser is:

<form>
  <input>
  <input type="password">
  <input type="email">
</form>

The type="text" attribute is getting stripped off the first input.

Without the type="text" attribute CSS rules that target input[type="text"] won't work.

brainkim commented 1 year ago

INVESTIGATING

brainkim commented 1 year ago

Okay my current working theory is that type="text" is the default, and the Crank DOM attribute/property algorithm is incorrectly flagging it as already set. @waynebaylor I am deeply sorry for any inconvenience this has caused πŸ™‡β€β™‚οΈπŸ™‡β€β™‚οΈπŸ™‡β€β™‚οΈ. As always thank you for the reproduction.

If my theory is correct, expect a fix in 0.5.4 within the hour. Otherwise, I will continue to investigate.

I’m slightly concussed from a bicycle accident right now but I am so glad you have attrited yet, Wayne!

brainkim commented 1 year ago

Okay. I figured out how to fix it but I am not going to, and I will explain why:

The default type of input elements is text (https://www.w3schools.com/tags/att_input_type.asp), so even if it looks wrong in a DOM inspector, it will actually behave as expected. You can double confirm by inspecting the element and logging inputEl.type.

Unless there are actual additional consequences which I am not aware of, this should be fine for the end user.

Nevertheless, if this kind of thing does bother you, I encourage you (or anyone) to make a PR! As a hint, the place to fix this would be in /dom.js under the method named β€œpatch.”

Again, thank you for opening an issue! BK

waynebaylor commented 1 year ago

Sorry to hear about the accident, hope you're feeling better!

I noticed this because a screen I'm working on has some CSS rules like:

input[type=email],
input[type=number],
input[type=password],
input[type=search],
input[type=text] {
 border:1px var(--input-style) var(--font-color);
 width:100%;
 padding:.7em .5em;
 font-size:1em;
 font-family:var(--font-stack);
 -webkit-appearance:none;
 border-radius:0
}

and the CSS wasn't getting applied to bare <input> elements.

Anyways, I'll take a look at dom.js and see if I can put together a PR :rocket:

brainkim commented 1 year ago

Ah yes CSS styling, durr on me. Will leave thoughts in PR.

brainkim commented 1 year ago

Shipped in 0.5.4!