Skip to content
This repository was archived by the owner on Jul 9, 2025. It is now read-only.

wip: add option delay to check pages #10

Draft
wants to merge 4 commits into
base: develop
Choose a base branch
from

Conversation

kip-13
Copy link
Collaborator

@kip-13 kip-13 commented Oct 31, 2023

PoC

image

Before
image

After
image

@kip-13
Copy link
Collaborator Author

kip-13 commented Oct 31, 2023

@roymckenzie, we could try to set another option as described here #2 🤔

Base automatically changed from bugfix/runtime-error to master October 31, 2023 15:25
@roymckenzie
Copy link
Owner

I saw your comments on the pull request you reviewed (thank you!), but am still not clear on why this error is being thrown. Is the DOM not fully loaded? Is setting an arbitrary delay going to cover edge cases?

@kip-13
Copy link
Collaborator Author

kip-13 commented Oct 31, 2023

The issue is we manipulate the DOM and when we use innerHTML the original nodes are kind of recreated and all the elements touched lose the original events or attrs created from any js code or framework, in this case for medium is react.

This possible solution is to set an arbitrary delay and with that wait for a time when all the js manipulation are done.

But I think perhaps we should change the innerHTML manipulation to something less intrusive 🤔

@roymckenzie
Copy link
Owner

Ah! I see. That makes sense. I dug around a bit. I tried to see if there was any way to set style rules based on the type of character. No dice.

I am open to seeing a different solution other than innerHTML to inject a span. Barring a new solution to injecting the span, I think a time delay is an acceptable solution.

I do not think it is necessary to allow the extension user to set the time. I think hardcoding a time delay is sufficient (wrapping the checkPage call in a setTimeout of 1000 milliseconds).

This will likely cover most edge cases until a cleaner solution can be found.

@roymckenzie roymckenzie changed the base branch from master to develop October 31, 2023 21:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants