-
Notifications
You must be signed in to change notification settings - Fork 48
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
Add cursorDidChange callback to editor compo #148
Conversation
@mattmcmanus Thanks for this PR. This makes sense to me to add. It would be nice not to have to punch through all these editor hooks directly (to make this addon easier to keep up-to-date with mobiledoc-kit changes), but that's definitely the pattern we have so far. Adding I am going to work on fixing the CI builds separately and return to merge this when I've sorted that out. |
That's a good thought. It would be pretty straightforward to wire up if kit's editor.js exported it's callback queue constants. The overlap in lifecycle names between it and ember's lifecycle hooks add some cognitive overhead. Maybe the callback method names are prefixed with Is that along the lines of what you're thinking? |
@mattmcmanus Sorry for the delay. I've fixed the build and I restarted the build for this PR. It might need a rebase, but if it goes green now I'll merge it straight away. |
@mattmcmanus Would you mind doing a rebase off master when you can? |
fa634e9
to
a7b23d9
Compare
@bantic Rebased. |
27395d2
to
30bf5af
Compare
@bantic I've rebased this and fixed the test failure |
@jasonbekolay and @mattmcmanus thank you both! We should also be sure to document this so that future travelers are aware of it. I'll add an issue. |
We've run into issues where the
inputModeDidChange
callback wasn't firing at times when we wanted it to.cursorDidChange
seems to do exactly what we want. We're implementing an interface that shows a link tooltip/prompt when you click on the link, rather than hover on it. I saw that you removed this in #69, so I'm not sure if you want it back or not.I'm up for discussion about how to implement and test this. We're doing most of our work in a subclass of
mobiledoc-editor
and so in this PR it's just setting up the callback and an empty method.Here is what our interface is looking like at the moment:
![mobiledoc-editor-tooltip](https://user-images.githubusercontent.com/9383/36736036-962a2178-1ba5-11e8-8538-76a346ab6913.gif)