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

inline event handling #12

Closed
thescientist13 opened this issue May 1, 2022 · 3 comments
Closed

inline event handling #12

thescientist13 opened this issue May 1, 2022 · 3 comments
Assignees
Labels
documentation Improvements or additions to documentation feature New feature or request question Further information is requested
Projects
Milestone

Comments

@thescientist13
Copy link
Member

thescientist13 commented May 1, 2022

Type of Change

  • Documentation / Website

Summary

Want to make sure that this pattern applies (assuming it should) to help ensure declarative event handling, e.g.

class Header extends HTMLElement {
  
  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.innerHTML = this.render();
    }
  }

  toggle() {
    alert('this.toggle clicked!');
  }

  render() {
    return `
      <header class="header">
        <button onClick="${this.toggle()}">Button To Click</button>
      </header>
    `;
  }
}

export { Header };
customElements.define('wcc-header', Header);

Details

The current example looks like this, which is more imperative.

class Header extends HTMLElement {
  
  connectedCallback() {
    if (!this.shadowRoot) {
      this.attachShadow({ mode: 'open' });
      this.shadowRoot.innerHTML = this.render();
    } else {
      const button = this.shadowRoot.querySelector('button');

      button.addEventListener('click', this.toggle);
    }
  }

  toggle() {
    alert('this.toggle clicked!');
  }

  render() {
    return `
      <header class="header">
        <button>Button To Click</button>
      </header>
    `;
  }
}

export { Header };
customElements.define('wcc-header', Header);

That said, I'm not sure if one is more or less useful for something like #11 ? Because if we want to strip out "dead code", which one is easer to detect via AST?

@thescientist13 thescientist13 added the feature New feature or request label May 1, 2022
@thescientist13 thescientist13 added this to the MVP milestone May 1, 2022
@thescientist13 thescientist13 self-assigned this May 1, 2022
@thescientist13 thescientist13 modified the milestones: MVP, 1.0 May 1, 2022
@thescientist13 thescientist13 added the documentation Improvements or additions to documentation label May 1, 2022
@thescientist13
Copy link
Member Author

An interesting thread, not sure if this is why with my initial testing it didn't work?
WICG/webcomponents#959

@thescientist13 thescientist13 added question Further information is requested and removed documentation Improvements or additions to documentation labels May 20, 2022
@thescientist13 thescientist13 added the good first issue Good for newcomers label Jun 5, 2022
@thescientist13 thescientist13 added this to TODO in 2022 Jun 5, 2022
@thescientist13 thescientist13 removed the good first issue Good for newcomers label Jun 14, 2022
@thescientist13
Copy link
Member Author

thescientist13 commented Jul 4, 2022

Ok, here's a very raw version of how it could be done inline. Not great to write but useful in the context of #69 👀

class Counter extends HTMLElement {   
 constructor() {
    super();
    this.count = 0;
  }

  connectedCallback() {
    this.render();
  }

  increment() {
    this.count += 1;
    this.render();
  }

  decrement() {
    this.count -= 1;
    this.render();
  }

  // TODO ideally a patch over a full re-render would be ideal
  // update() {
  //   this.querySelector('span#count').textContent = this.count;
  // }

  render() {
    const { count } = this;
  
    // parentElement can be reverse engineered by walking up from this to wcc-counter
    // in this case 2; button -> div -> wcc-counter
    this.innerHTML = `
      <div>
        <button onclick="this.parentElement.parentElement.decrement();"> + </button>
        <span>You have clicked <span id="count">${count}</span> times</span>
        <button id="inc" onclick="this.parentElement.parentElement.increment();"> + </button>
      </div> 
    `;
  }
}

customElements.define('wcc-counter', Counter);

Technically the update function can be removed now since the callbacks are inlined. This is nice because when setting innerHTML like this each render call, the event handlers will get blown away with the existing DOM and so the event handler will only work once. Just an FYI.

But obviously if we can just patch the dynamic parts a la Lit, then that would be 👌


With Shadow DOM, hoping document could refer to <wcc-counter> and so we could even refine it down to this? 🤔

  render() {
    const { count } = this;
  
    this.innerHTML = `
      <div>
        <button onclick="this.shadowRoot.decrement();"> + </button>
        <span>You have clicked <span id="count">${count}</span> times</span>
        <button onclick="this.shadowRoot.decrement();"> + </button>
      </div> 
    `;
  }

@thescientist13 thescientist13 moved this from TODO to IN PROGRESS in 2022 Jul 4, 2022
@thescientist13 thescientist13 moved this from IN PROGRESS to IN REVIEW in 2022 Jul 4, 2022
@thescientist13 thescientist13 added the documentation Improvements or additions to documentation label Jul 4, 2022
@thescientist13
Copy link
Member Author

Nothing really to do here, unless we want to support something like this outside of JSX?

@thescientist13 thescientist13 moved this from IN REVIEW to DONE in 2022 Nov 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation feature New feature or request question Further information is requested
Projects
No open projects
2022
DONE
Development

No branches or pull requests

1 participant