Checkbox inputs #125
Checkbox inputs #125
Conversation
There was a problem hiding this comment.
They should be drawing their colors using the css property 'accent-color', but I couldnt figure out how to look that up, so they're using just 'color' for now.
See comments on relevant code.
There's something up with the vertical alignment when rendering labels with the text 'inline'. Check the second checkbox in the example, or try removing the 'inline-block' from label styling in the css. In chrome and ff, the labels render the same vertically whether inline or inline-block.
Weird that the two checkboxes are rendering differently, but let's not worry too much about this for now.
The FormEvent has valid: false, as its calculated as 'true' if value is empty? However I've set value to the value of the toggled element. Not sure if this is correct
I believe it should be the literal on if the value is empty. Saw that in an MDN doc somewhere.
Checking a checkbox send 'onclick' and 'oninput' events. Not sure if it should send 'onchanged' too? Didnt see any mention of that event in dioxus docs.
I believe browsers send onchanged for checkboxes, so we probably should too.
Some nice to haves (non-blocking):
- Implementing the
disabledattribute, which would prevent clicks from taking effect and would also make the checkbox render with a greyed out look. - Making the space bar toggle the checkbox if it is currently focussed.
| return compute_leaf_layout( | ||
| inputs, | ||
| &node.style, | ||
| |_known_size, _available_space| { |
There was a problem hiding this comment.
I suspect that the vertical alignment issue you are seeing is due to taffy/parley assuming the bottom of the node's border-box as it's baseline. Whereas for checkboxes it probably ought to ignore the margin and use the bottom of the actual checkbox as the baseline.
We will need to add support for inline-box baselines to parley to make that work properly, but a good first step would be to modify the first_baselines.y field of the taffy::LayoutOutput returned by compute_leaf_layout to set the baseline to the bottom of the checkbox (height of checkbox + top padding/border/margin).
| self.tree() | ||
| .into_iter() | ||
| .filter_map(|(_id, node)| { | ||
| let element_data = node.element_data()?; | ||
| if element_data.name.local != local_name!("input") { | ||
| return None; | ||
| } | ||
| let id = element_data.id.as_ref()?; | ||
| if *id == *target_element_dom_id { | ||
| Some(node) | ||
| } else { | ||
| None | ||
| } | ||
| }) | ||
| .collect() |
There was a problem hiding this comment.
We probably ought to maintain a global ID map so this can be looked up much more quickly? I wouldn't block this PR on that, but perhaps an "iterate over nodes with a given id" method could be added to Document?
| .and_then(|c| c.parse().ok()) | ||
| .unwrap_or_default(); | ||
|
|
||
| // TODO this should be coming from css accent-color, but I couldn't find how to retrieve it |
There was a problem hiding this comment.
accent-color can be enabled here: https://github.com/servo/stylo/blob/main/style/properties/longhands/inherited_ui.mako.rs#L94 by changing engines="gecko" to engines="gecko servo".
Such a change could be submitted to the DioxusLabs fork of Stylo. We are currently using the enable-table-moz-center-style-adjust branch. So changes should be based on that branch (we should probably be more principled about using a blitz branch or similar).
| let checked: bool = self | ||
| .element | ||
| .attr(local_name!("checked")) | ||
| .and_then(|c| c.parse().ok()) | ||
| .unwrap_or_default(); |
There was a problem hiding this comment.
checked is a boolean attribute, so this should just check for the presence of the checked attribute in the array. If it is present then checked is true else false (even the empty string or "false" counts as true).
See: https://developer.mozilla.org/en-US/docs/Glossary/Boolean/HTML
| pub fn toggle_checkbox(el: &mut ElementNodeData) { | ||
| let checked_attr_opt = el | ||
| .attrs | ||
| .iter_mut() | ||
| .find(|attr| attr.name.local == local_name!("checked")); | ||
|
|
||
| let checked_attr = if let Some(attr) = checked_attr_opt { | ||
| attr | ||
| } else { | ||
| let attr = Attribute { | ||
| name: QualName::new(None, ns!(html), local_name!("checked")), | ||
| value: String::from("false"), | ||
| }; | ||
| el.attrs.push(attr); | ||
| el.attrs | ||
| .iter_mut() | ||
| .find(|attr| attr.name.local == local_name!("checked")) | ||
| .unwrap() | ||
| }; | ||
| let checked = checked_attr.value.parse().unwrap_or(false); | ||
| checked_attr.value = (!checked).to_string(); | ||
| } |
There was a problem hiding this comment.
This needs to actually remove the attribute entirely when unchecking and add it when checking. I suggest using the empty string for the value to avoid an allocation.
| self.set_focus_to(hit.node_id); | ||
| } | ||
| // Clicking labels triggers click, and possibly input event, of associated input | ||
| else if el.name.local == local_name!("label") { |
There was a problem hiding this comment.
Nice that you've implemented label!
|
This PR seems to be breaking button layout on the
This PR: 
|
Adds support for input type="checkbox", as requested in #94 . Makes checkboxes send
oninputevent when clicked. If event isn't bound (there's no data-dioxus-id), they will behave as an unmanaged input, and handle checked state internally. If it is, they will send an oninput event with a FormData argument.Adds a 'form' example to demonstrate.
Caveats:
valid: false, as its calculated as 'true' if value is empty? However I've set value to the value of the toggled element. Not sure if this is correct