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

Use innerHTML instead of innertText on buttonTarget #7

Closed
letibelim opened this issue Oct 11, 2022 · 0 comments · Fixed by #8
Closed

Use innerHTML instead of innertText on buttonTarget #7

letibelim opened this issue Oct 11, 2022 · 0 comments · Fixed by #8

Comments

@letibelim
Copy link

Hey there,

In the controller, innerText is used to retrieve and put back the content of the buttonTarget in the connect and copied methods:

connect (): void {
    if (!this.hasButtonTarget) return

    this.originalText = this.buttonTarget.innerText
  }
copied (): void {
  ...
  this.timeout = setTimeout(() => {
      this.buttonTarget.innerText = this.originalText
    }, this.successDurationValue)
  }

Therefore if the content is, let's say, an icon, it doesn't work anymore. Wouldn't it be a nice additional feature to user innerHTML so that any kind of content would be put back in place once the successContent message has been displayed, like the suggestion below ? I can't see any side effect as it would work the same for simple text and only extend the component capabilities, but I might miss something...

connect (): void {
    if (!this.hasButtonTarget) return

    this.originalHTML= this.buttonTarget.innerHTML
  }
copied (): void {
  ...
  this.timeout = setTimeout(() => {
      this.buttonTarget.innerHTML = this.originalHTML
    }, this.successDurationValue)
  }

Thanks,

mkrauser added a commit to mkrauser/stimulus-clipboard that referenced this issue Oct 27, 2022
This fixes stimulus-components#7 and makes it possible, to use buttons with icon only.

I was unsure, if we should change it for the successContent as well. What do you think?
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant