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

Visual diff: trigger DOM_CHANGED event after enable/disable #465

Merged
merged 5 commits into from
Dec 10, 2024

Conversation

humitos
Copy link
Member

@humitos humitos commented Dec 5, 2024

A new event READTHEDOCS_ROOT_DOM_CHANGED is triggered when the "Visual diff" is enabled/disabled after the DOM was modified.

This allows other addons to subscribe to this event to re-initialize if required. I used this event from links preview to call setupTooltips again to re-install tooltips on these links.

I migrated links preview to a LitElement to be able to make usage of this pattern and follow what we already had.

We will be able to do something similar on #157

Closes #460

A new event `READTHEDOCS_ROOT_DOM_CHANGED` is triggered when the "Visual diff"
is enabled/disabled after the DOM was modified.

This allows other addons to subscribe to this event to re-initialize if
required. I used this event from links preview to call `setupTooltips` again to
re-install tooltips on these links.

I migrated links preview to a `LitElement` to be able to make usage of this
pattern and follow what we already had.

We will be able to do something similar on #157
@humitos humitos requested a review from a team as a code owner December 5, 2024 15:40
@humitos humitos requested a review from agjohnson December 5, 2024 15:40
@humitos humitos changed the base branch from main to humitos/docdiff-parameter-event December 5, 2024 15:40
@humitos humitos changed the base branch from humitos/docdiff-parameter-event to main December 5, 2024 15:41
ericholscher pushed a commit that referenced this pull request Dec 9, 2024
HotKeys has to be initialized before DocDiff because when
`?readthedocs-diff=true` DocDiff triggers an event that HotKeys has to
listen to to update its internal state.

* Related #465
* Related #441
Copy link
Member

@ericholscher ericholscher left a comment

Choose a reason for hiding this comment

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

Looks like a nice refactor to me.

@@ -9,6 +9,8 @@ export const EVENT_READTHEDOCS_FLYOUT_SHOW = "readthedocs-flyout-show";
export const EVENT_READTHEDOCS_FLYOUT_HIDE = "readthedocs-flyout-hide";
export const EVENT_READTHEDOCS_ADDONS_DATA_READY =
"readthedocs-addons-data-ready";
export const EVENT_READTHEDOCS_ROOT_DOM_CHANGED =
Copy link
Member

Choose a reason for hiding this comment

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

Should we have a docstring or similar to describe the events here?

Copy link
Member Author

Choose a reason for hiding this comment

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

Added 👍🏼

@humitos humitos merged commit a89136b into main Dec 10, 2024
4 checks passed
@humitos humitos deleted the humitos/docdiff-dom-changed-event branch December 10, 2024 11:03
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.

Passing ?readthedocs-diff=true breaks Links Preview
2 participants