Skip to content

Add a warning to the JS Hook documentation for changing phx-hook#3509

Closed
Gazler wants to merge 1 commit into
mainfrom
gr-hook-docs
Closed

Add a warning to the JS Hook documentation for changing phx-hook#3509
Gazler wants to merge 1 commit into
mainfrom
gr-hook-docs

Conversation

@Gazler

@Gazler Gazler commented Nov 15, 2024

Copy link
Copy Markdown
Member

No description provided.

@SteffenDE

Copy link
Copy Markdown
Member

@Gazler I think we could also make this work instead of just documenting that it doesn't

> ```javascript
> hooks.MyHook = {
> updated: {
> if (this.el.getAttribute("data-hook-enabled") !== "false")) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
> if (this.el.getAttribute("data-hook-enabled") !== "false")) {
> if (this.el.dataset.hookEnabled !== "false") {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i prefer "data-hook-enabled" for the fact being is much easier to search for it globally. There is no benefit of one over the other.

@Gazler Gazler closed this Apr 16, 2026
@SteffenDE

Copy link
Copy Markdown
Member

fwiw there's #4153 which would allow changing hooks

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 this pull request may close these issues.

4 participants