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

Docdiff should not apply the 'd' hotkey in textareas #205

Closed
davidfischer opened this issue Nov 27, 2023 · 4 comments · Fixed by #206
Closed

Docdiff should not apply the 'd' hotkey in textareas #205

davidfischer opened this issue Nov 27, 2023 · 4 comments · Fixed by #206
Assignees
Labels
Accepted Accepted issue on our roadmap Bug A bug

Comments

@davidfischer
Copy link
Contributor

Docdiff doesn't apply the d hotkey when typing in <input> fields but should apply similar checks to <textarea> fields.

addons/src/hotkeys.js

Lines 49 to 69 in ff8baf2

_handleKeydown = (e) => {
// Close docdiff with single-stroke `d` (no Ctrl, no Shift, no Alt and no Meta)
// (I'm checking `document.activeElement` to check if it not inside an INPUT to avoid enable/disable while typing on forms)
// Read more about these decisions at https://github.com/readthedocs/addons/issues/80
let event;
// DocDiff
if (
this.docDiffHotKeyEnabled &&
keyboardEventToString(e) ===
this.config.addons.hotkeys.doc_diff.trigger &&
document.activeElement.tagName !== "INPUT"
) {
if (this.docDiffShowed) {
event = new CustomEvent(EVENT_READTHEDOCS_DOCDIFF_HIDE);
this.docDiffShowed = false;
} else {
event = new CustomEvent(EVENT_READTHEDOCS_DOCDIFF_ADDED_REMOVED_SHOW);
this.docDiffShowed = true;
}
}

You can see this bug in the notes field on ethicalads.io where the 'd' character cannot be typed.

@humitos humitos added Bug A bug Accepted Accepted issue on our roadmap labels Nov 28, 2023
@github-project-automation github-project-automation bot moved this to Planned in 📍Roadmap Nov 28, 2023
@humitos humitos moved this from Planned to Needs review in 📍Roadmap Nov 28, 2023
@humitos humitos self-assigned this Nov 28, 2023
@github-project-automation github-project-automation bot moved this from Needs review to Done in 📍Roadmap Nov 28, 2023
@astrojuanlu
Copy link

We're still spotting this kedro-org/kedro#3371 (comment)

@humitos
Copy link
Member

humitos commented Nov 30, 2023

@astrojuanlu thanks for commenting here pointing out to a reproducible example. I think the issue you are hitting is similar to the one this issue mentions and it was solved in v0.9.0 by this PR of the addons which I released the past Tuesday but I had to revert that deploy because it broke other stuffs 😅 (see #207)

I double checked the code and it seems it's correct: https://github.com/readthedocs/addons/blob/main/src/hotkeys.js#L58-L63

Once I fix the 0.9.0, I will do another one that once deployed it should solve the issue you are experimenting. I'm targeting this release to next Tuesday. If you want, I can disable the "hotkeys addons" in your project now as an immediate workaround. Let me know.

I'll keep you posted otherwise 👍🏼

@astrojuanlu
Copy link

Thanks! cc @rashidakanchwala

@humitos
Copy link
Member

humitos commented Nov 30, 2023

I just merged #208 which solves the issue with the previous release and I've deployed 0.9.1 with the fix for this. I just tested your project at https://rashida-test-kedro.readthedocs.io/en/latest/ and I was able to type the d letter without problem when performing a search 😄

I'd appreciate any other feedback you may have in the future 🙏🏼

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Accepted Accepted issue on our roadmap Bug A bug
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

3 participants