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

[RFC] Ability to not double specify on bindable match on name #1259

Closed
brandonseydel opened this issue Jul 23, 2021 · 18 comments · Fixed by #1963
Closed

[RFC] Ability to not double specify on bindable match on name #1259

brandonseydel opened this issue Jul 23, 2021 · 18 comments · Fixed by #1963
Labels
RFC Request For Comment - Potential enhancement that needs discussion and/or requires experimentation

Comments

@brandonseydel
Copy link
Member

💬 RFC

Allow user to ignore specifying the same property name if matched on the attribute binding to.

🔦 Context

This would shrink html and make more readable

💻 Examples

class TestCE {
@bindable value:string;
}

class App {
value = 'test'
}

Current State

<test-ce value.bind="value" />

Proposed

<test-ce value.bind  />
// or using shorthand
<test-ce :value />
@brandonseydel brandonseydel changed the title Ability to not double specify on bindable match on name [RFC] Ability to not double specify on bindable match on name Jul 23, 2021
@brandonseydel brandonseydel added the RFC Request For Comment - Potential enhancement that needs discussion and/or requires experimentation label Jul 23, 2021
@bigopon
Copy link
Member

bigopon commented Jul 24, 2021

At the moment, we already have the short hand <test-ce value.bind /> working. :value could be enhance to make it work if needed. We just need to transfer more power from the compiler to binding command so that it takes care of everything instead of the compiler determining the expression then parsing it, then pass that parsed expression to the binding command.

@bigopon
Copy link
Member

bigopon commented Jul 25, 2021

What we could consider further, is to apply the same shorthand for normal bindings as well:

<input value.bind="value">

Is it good to enable:

<input value.bind >

@zewa666
Copy link
Member

zewa666 commented Jul 25, 2021

So would that include value.one-time and value.two-way as well?

@brandonseydel
Copy link
Member Author

What we could consider further, is to apply the same shorthand for normal bindings as well:

<input value.bind="value">

Is it good to enable:

<input value.bind >

Yes we should apply on shorthand and @zewa666 stuff should be naturally included

@bigopon
Copy link
Member

bigopon commented Jul 28, 2021

If we are to allow shorthand everywhere, what should we do with the special case:

<td colspan.bind>

because colspan attribute is mapped to colSpan property, should the above binding be understood as:

  1. WYSIWYG:
<td colspan.bind="colspan">

or
2. What the mapped property is

<td colspan.bind="colSpan">

Similar to other properties like textcontent, innerhtml, minlength etc...

Note that we are camel-casing the prop for shorthand bindable on custom element:

<name-tag first-name.bind>

equals:

<name-tag first-name.bind="firstName">

@avrahamcool
Copy link
Sponsor Contributor

@bigopon
in order to better understand the issue:
why is the colSpan attribute accessible via colspan in the first place,
shouldn't it be col-span instead?

(same goes for the other examples).

if that were the case, it would behave exactly how you explained for firstName.

<td col-span.bind> => <td col-span.bind="colSpan">

I think <input value.bind > is a great addition. (as well as two-way etc.)
IMO, most of the bindings are in the form of x.bind="x".

@bigopon
Copy link
Member

bigopon commented Jul 30, 2021

why is the colSpan attribute accessible via colspan in the first place,

It's a productivity helper. Writing col-span.bind works without any extra work. But there are things that aren't that fun to write:

<div inner-h-t-m-l.bind="myHtml">

compared to

<div innerhtml.bind="myHtml">

Maybe we don't have to provide all the mapping, just only special one for innerHTML, that could also be an option, so just only 1 rule to remember: everything is camel-case-ised. This would be a big breaking change from v1 template though.

@avrahamcool
Copy link
Sponsor Contributor

I think the correct kebab-case transformation of innerHTML is inner-html.
I tried several online converters, some agree with me, some agree with you.

inner-html:
https://textedit.tools/kebabcase
https://www.appdevtools.com/case-converter

inner-h-t-m-l:
https://www.better-converter.com/Case-Converters/Kebab-Case-Conterter

I'll try to dig into the official definition later.

but regardless of that, I agree with you that 1 rule to remember is the way to go.
in regard of the breaking from V1: this part was always a bit clanky in V1, it was just stuff to remember (and not very well documented) - I had a lot of co-workers who struggled with that bit.
a clean rule like "just kebab it" would be very good (as this is already a rule in the template for everything else).

@Vheissu
Copy link
Member

Vheissu commented Aug 3, 2021

I remember raising this about a year ago and I could have sworn that we had this already. Is that not the case currently @bigopon ?

@bigopon
Copy link
Member

bigopon commented Aug 3, 2021

It is the case for bindable properties. This is to consider making it available everywhere. Probably custom attribute is an exception:

<div square.bind>

Should be understood as

<div square>

(no value /empty string)

@avrahamcool
Copy link
Sponsor Contributor

It is the case for bindable properties. This is to consider making it available everywhere. Probably custom attribute is an exception:

<div square.bind>

Should be understood as

<div square>

(no value /empty string)

Why is the case of custom attribute different?
It's seems benefitial to have the same rule apply across all similar syntax.
If the user don't want to bind anything, why use the bind keyword at all? And if he wants to bind to an empty string he can use <div square.bind=''>.

@bigopon
Copy link
Member

bigopon commented Aug 3, 2021

<div square.bind=''>

is actually the same with

<div square.bind>

Why is the case of custom attribute different?

Im not 100% sure on this, a v1 behavior carried over. Maybe we don't have to/shouldn't keep it.

@bigopon
Copy link
Member

bigopon commented Aug 9, 2021

@avrahamcool I just came across another example while we a mapping may look good: the for attribute on <label> element is mapped to htmlFor property of it. Do we want to apps to always have:

<label html-for.bind="...">

or just

<label for.bind="...">

@zewa666
Copy link
Member

zewa666 commented Aug 9, 2021

Isnt this similar to class and className?

@avrahamcool
Copy link
Sponsor Contributor

@avrahamcool I just came across another example while we a mapping may look good: the for attribute on <label> element is mapped to htmlFor property of it. Do we want to apps to always have:

<label html-for.bind="...">

or just

<label for.bind="...">

Isnt this similar to class and className?

you are right. you convinced me that we must have a map of some sort.
as developer - we often think in terms of HTML tags attributes, and not always in JS properties.

I guess that <label html-for.bind="..."> should work by default, as it represent a JS property. am I right?

@bigopon
Copy link
Member

bigopon commented Aug 9, 2021

Isnt this similar to class and className?

I guess that should work by default, as it represent a JS property. am I right?

My common comment for both of these 2 comments is that our template should look like html, instead of a mix of html + JS, unless it's explicitly done so by the devs.

I guess that should work by default, as it represent a JS property. am I right?

Yes, everything is camel-case-ized by default, and html-for will be as htmlFor, and should just work.

@bigopon
Copy link
Member

bigopon commented May 3, 2023

We can start implementing this, some simple tweaks in the logic should do. Thanks @tomtomau for asking about this in #1758

@Sayan751
Copy link
Member

@bigopon AFAIK, the proposed shorthand in the original post already works. Can this be closed?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
RFC Request For Comment - Potential enhancement that needs discussion and/or requires experimentation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants