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

Allow false values on data-attributes #1

Closed
4 tasks
ThijsTyZ opened this issue Oct 5, 2021 · 1 comment · Fixed by #4
Closed
4 tasks

Allow false values on data-attributes #1

ThijsTyZ opened this issue Oct 5, 2021 · 1 comment · Fixed by #4
Assignees
Labels
good first issue Good for newcomers

Comments

@ThijsTyZ
Copy link

ThijsTyZ commented Oct 5, 2021

By default, html attributes that have the false value are omitted completely in the resulting HTML. Specifically for data-attributes we want to prevent this; we like explicit true/false values to be there – allowing the default value to take affect when it's missing.

Inside the following function;

function h(type: any, props: Record<string, any>, ...children: Array<any>) {
// TODO: add logic for props to mutate them based on possible functions
return vhtml(type, props, children);
}
we can change the parsed AST before it renders to a string. Every HTML node is passed through that function, with its name, props, and children.

For any node we can loop over the props, and if its name starts with data- and the value is === false, we can change the value to "false".

Tasks:

  • write unit tests that fail on the current implementation
  • implement the feature, fixing the unit tests
  • document the change in the readme as an adaptation from the original library
  • update the documentation in the main muban repo

Example test:

expect(html`<div data-test=${false}>foo</div>`).toEqual('<div data-test="false">foo</div>');

Original request

The propType.boolean should not have the option to set a default value. Because if a boolean prop has the default value of true, it won't be possible to set the value back to false. This is due to the way how Muban renders the HTML:

html`<div test=${false} ></div>;

will be rendered as:

<div></div>

as false props are not added to the HTML.

@ThaNarie
Copy link
Contributor

  1. Default values for booleans can be useful when passing them as component props, extracting from HTML is not the only use case.
  2. There is a workaround for the above, using <div test=${String(false)}></div>
  3. The test=${false} might only work like that in muban templates, and CMS template engines might render it properly with a "false" value.

I would rather find a way to disable the auto-removal of those boolean properties when there are data-attributes.

So that could then work like this:

html`<input />`; // false
html`<input checked />`; // true
html`<div data-active="false" />`; // false
html`<div data-active="true" />`; // true

This could be done relatively straight-forward in the muban-template package by modifying the parsed props before rendering the HTML; when the property-name begins with data-, and the value is false, change the value to "false".

How do you feel about that?

@ThaNarie ThaNarie transferred this issue from mubanjs/muban Nov 17, 2021
@ThaNarie ThaNarie changed the title Remove defaultValue from propType.boolean Allow false values on data-attributes Nov 17, 2021
@ThaNarie ThaNarie added the good first issue Good for newcomers label Jan 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants