Skip to content
This repository was archived by the owner on Apr 6, 2018. It is now read-only.

Use TextEditorElement for VimCommandModeInputView#582

Merged
maxbrunsfeld merged 5 commits into
masterfrom
fix-search-input
Mar 19, 2015
Merged

Use TextEditorElement for VimCommandModeInputView#582
maxbrunsfeld merged 5 commits into
masterfrom
fix-search-input

Conversation

@tmm1
Copy link
Copy Markdown
Contributor

@tmm1 tmm1 commented Mar 14, 2015

fixes a bug where you're unable to see the text you're typing when hitting / to search

searching is working on this PR, but I introduced a regression into Motion.Till where the search input box shows up whereas previously it was invisible

cc @maxbrunsfeld

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

The hidden-input class was a special case in the deprecated TextEditorView. I'm not sure what the equivalent is for TextEditorElement

@MattKunze
Copy link
Copy Markdown

fwiw I have this updated in #545

@maxbrunsfeld
Copy link
Copy Markdown
Contributor

@MattKunze the nice thing about this approach is that it uses the newer TextEditorElement class, which avoids having to use atom-space-pen-views. That module will work, but it depends on space-pen and jQuery, so it'd be nice to use TextEditorElement directly instead. It just means we need to do a little more work to update the tests to not use the jQuery interface.

@tmm1 did you still want to work on this? If not, I can pick it up here. I don't think the hidden-input thing needs to change. That class is being applied to a div that contains the text editor element, not the element itself, so I would think it would continue to work.

If you're not seeing it have any effect, it might already have been broken.

@MattKunze
Copy link
Copy Markdown

It didn't seem like the optimal approach, I was just following the bouncing balls of documentation regarding upgrading the internal views.

@tmm1
Copy link
Copy Markdown
Contributor Author

tmm1 commented Mar 17, 2015

Feel free to pick this up. I noticed the old deprecated class had some special handling for hidden-input, and I wasn't able to make the new view hidden.

@MattKunze
Copy link
Copy Markdown

I think the motion regression is with handling single-character inputs. The input used to show up, but it was closed after the first character that was typed. That's what hung me up on the other PR was how to handle that with the updated internals.

@tmm1
Copy link
Copy Markdown
Contributor Author

tmm1 commented Mar 17, 2015

The input used to show up, but it was closed after the first character that was typed.

In my testing it never used to show up. There's code in the plugin to explicitly hide it in the singleChar case.

On this PR, it does show up and it goes away as soon as you type a character. But it's awkward to see it pop up briefly and definitely distracting.

@maxbrunsfeld
Copy link
Copy Markdown
Contributor

Fixes #544

@maxbrunsfeld maxbrunsfeld changed the title [WIP] use TextEditorElement for VimCommandModeInputView Use TextEditorElement for VimCommandModeInputView Mar 18, 2015
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

This doesn't seem to work with addEventListener. Maybe the event needs to be keyup or something different

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.

Yeah, I can't remember why that doesn't work. Observing text changes on the model seems to work ok though.

maxbrunsfeld pushed a commit that referenced this pull request Mar 19, 2015
Use TextEditorElement for VimCommandModeInputView
@maxbrunsfeld maxbrunsfeld merged commit 747f594 into master Mar 19, 2015
@maxbrunsfeld maxbrunsfeld deleted the fix-search-input branch March 19, 2015 16:22
@tmm1
Copy link
Copy Markdown
Contributor Author

tmm1 commented Mar 20, 2015

Thanks @maxbrunsfeld, this is working great now!

MarkusSN pushed a commit to MarkusSN/vim-mode that referenced this pull request May 28, 2015
Use TextEditorElement for VimCommandModeInputView
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.

3 participants