-
-
Notifications
You must be signed in to change notification settings - Fork 115
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
Some sites may remove listeners on blur
#213
Comments
Thanks for the video! Indeed GhostText/source/ghost-text.js Lines 164 to 174 in ad87dff
|
I guess it makes sense for it to be the last one dispatched. What do you think? |
Heh, the code I posted earlier seems to be the problem. It's not GhostText but GitHub itself is removing the event handler on You can see the "Close with comment" keeps working even in Chrome. To fix this I should trigger |
Maybe Safari and Chrome are doing different things? For me adding the |
Ok, I've been debugging GitHub's code and you're absolutely right, I ended up in the same place: // Observe required fields and validate form when their validation state
// changes.
onFocus(supportedFields, field => {
let previousValid = (field as Field).checkValidity()
function inputHandler() {
const currentValid = (field as Field).checkValidity()
if (currentValid !== previousValid && (field as Field).form) {
validate((field as Field).form!)
}
previousValid = currentValid
}
field.addEventListener('input', inputHandler)
field.addEventListener('blur', function blurHandler() {
field.removeEventListener('input', inputHandler)
field.removeEventListener('blur', blurHandler)
})
}) I'll try to see if I can see why adding the |
Ok, saw it. It's because there is also another listener in the form for // Install validation handlers on a form.
function installHandlers(form: HTMLFormElement) {
if (installedForms.get(form)) return
form.addEventListener('change', () => validate(form))
installedForms.set(form, true)
}
// Validate a form or input.
export function validate(form: HTMLInputElement | HTMLFormElement) {
const validity = form.checkValidity()
for (const button of form.querySelectorAll<HTMLButtonElement>('button[data-disable-invalid]')) {
button.disabled = !validity
}
} So it is listening to the I agree with you that sending a Any other ideas? 🙂 |
Other than re-focusing the field I don’t think that there's anything else. Maybe preventing the first blur event via stopImmediatePropagation? |
In my opinion both sound a bit hacky. Sorry, I thought this had a simple fix. The only solution that seems rather "safe" to me is to apply this fix only for |
The |
Hmm... I guess the problem is when to retrigger it again, don't you think? If we just swallow the |
Once the user returns to the browser, the field will be focused again and the regular flow will resume. Either blocking the
I think it's easier/safer to avoid the first blur altogether. In this specific instance the issue is that events are disconnected on |
Ok, I guess it's worth a try 🙂 Do you know of any other sites that depend on this apart from GitHub? |
blur
Setup
Browser: Chrome
Editor: VS Code
Description
Sites that listen to the
change
event of the textarea don't update.One example is GitHub's "Comment" button of issues and PRs, which doesn't update when the text is written via GhostText.
ScreenFlow.mp4
I'll open a PR to fix it.
The text was updated successfully, but these errors were encountered: