buschtoens / ember-link

Link primitive to pass around self-contained route references. It's {{link-to}}, but better!
https://buschtoens.github.io/ember-link/
Other
54 stars 12 forks source link

Link modifier? #333

Open gossi opened 4 years ago

gossi commented 4 years ago

I was wondering if a modifier for situations like this would be a good idea:

<MyComponentWhereIDontKnowTheElementFor {{link ...}}/>

the {{link}} modifier would turn the element into an <a> tag and apply its values. Passing around the link helper as primitive would still work as the modifier would take the helper as valid input although it really looks stupid: {{link (link ...)}} 😂

The idea: With only the helper each component need to be made aware of a link passed in as helper and then handle the logic of applying it or pass it down. The modifier eliminates this component-specific-handling and applies it to where it is attached, some examples:

<Button>foo</Button> 
-> <button class="_button1234">foo</button>

<Button {{link ...}}>foo</Button> 
-> <a href="..." class="_button1234">foo</a>

<Card>bar</Card> 
-> <div class="_card1744">bar</div>

<Card {{link ...}}>bar</Card> 
-> <a href="..." class="_card1744">bar</a>

WDYT?

buschtoens commented 4 years ago

An element modifier cannot simply change the element tag name and I don't think that this is desirable, since it's quite magical and non-obvious / unexpected. In your template you'd write:

<div {{link ...}}>...</div>

But the result would be:

<a href="...">...</a>

There is ember-element-helper already, which was made exactly for this job.

In general, I recommend to treat ember-link as a first class primitive in your app architecture that is on-par with click actions. You could create a common base link button component, that accepts an optional @onClick and optional @link argument and changes its tag accordingly and wires up the arguments. We could think about adding something like this instead:

<OptionalButton @nonInteractiveTagName="div" />
➡️ <div></div>

<OptionalButton @nonInteractiveTagName="div" @onClick={{fn ...}} />
➡️ <button></button>    

<OptionalButton @nonInteractiveTagName="div" @link={{link ...}} />
➡️ <a href="..."></a>

<OptionalButton @nonInteractiveTagName="div" @link={{link ...}} @onClick={{fn ...}} />
➡️ <a href="..."></a>
gossi commented 4 years ago

I made a PoC to see if changing the element would work. Well it does to what I expected it to do: https://ember-twiddle.com/b8b77f000ae29d52534b10936c4c4674?openFiles=templates.application%5C.hbs%2C&route=%2Fpage-1

See the difference between the links on the top and the bottom. The ones on the bottom ignore the listeneres attached with the {{on}} modifier. Dunno what's the order of execution of modifiers here? (perhaps by registration order in the modifier manager). There are solutions to that:

That said, I haven't discarded the idea. Because of the nice symmetry of building blocks the modifier and helper would offer: One to pass it around, the other to put it onto the element (with its clear purpose of changing the element). That would eliminate the fact, that components need to be made aware of taking a @llink arg and can handle it or not.

cah-brian-gantzler commented 4 years ago

I would think the desired action would be achieved having the current component support an inline version that instead of yields does tag. with appropriate use of ...attributes it should accomplish the same thing.

I would vote of a default implementation when not yeilding.

buschtoens commented 4 years ago

@gossi FWIW I added an attribute to the switched element you applied the modifier to to show that it breaks the internal Glimmer references and templates updates. Look at the tags in the DevTools and check the data-counter attribute that gets incremented when clicking the button. Twiddle

I really don't think that we should try to hack this deep into the Glimmer internals. If there's no public API way, we shouldn't do it.

@cah-briangantzler Hm, when I think this through it doesn't seem to make sense. I'd like to confirm, that I understood you right. You're proposing allowing a block-less invocation like this:

<Link
  @route="some.route"
  @models={{array 123}}
  @query={{hash foo="bar"}}
/>

And this should then yield some default HTML, which I guess should be an <a ...attributes> tag? Immediate questions that arise for me are:

buschtoens commented 4 years ago

Coming back to the <OptionalButton> or maybe rather <LinkButton> (or whatever name 😅) from https://github.com/buschtoens/ember-link/issues/333#issuecomment-626687314. The implementation would be straightforward and could even be done as a template-only component:

{{#if @link~}}
  <a
    href={{@link.url}}
    {{on "click" (if @replace @link.replaceWith @link.transitionTo)}}
    {{on "click" (optional @onClick)}}
    ...attributes
  >
    {{~yield~}}
  </a>
{{~else if @onClick~}}
  <button {{on "click" @onClick}} ...attributes>
    {{~yield~}}
  </button>
{{~else~}}
  {{~#let (element (or @nonInteractiveTageName "span")) as |Tag|~}}
    <Tag ...attributes>{{yield}}</Tag>
  {{~/let~}}
{{~/if}}

(The ~ remove the leading / trailing white space.)

cah-brian-gantzler commented 4 years ago

Yes, you would have to supply some of the click and default stuff, then the ...attributes would allow for overriding or applying additional things (like class).

I just found this and had not fully understood it so yes, you are right, you would have to solve the innerHTML which is looks like you have some good ideas.

Being a modifier is not the way to go, just agreeing with you.

the ~ is a little known and used handlebars syntax, have know about it for a while