sproutcore / rich-text-editor

A rich text editor for SproutCore.
Other
6 stars 6 forks source link

Allows for link clicks to trigger when editor is not first responder #24

Closed mysterlune closed 9 years ago

mysterlune commented 9 years ago

Allows for link clicks to trigger a window.open call to target _blank. Currently, clicking a link simply puts the editor into isFirstResponder, which is not consistent with user expectation.

The approach here is to simply modify the editor's mouseDown event handler to detect whether the event.target is an anchor tag AND whether the editor has isFirstResponder.

A user story here might be "As a user, if I click a link in the editor while NOT editing content, the link should open a new window/tab and display the linked webpage." And "... if I click a link while IN editing mode, place the caret within the link text so that I can modify the link using the toolbar tool, if needed."

Unit test provided.

mysterlune commented 9 years ago

@publickeating, thanks for the feedback. I appreciate the pointers in the unit test.

I wasn't sure if I entirely understood what you were meaning with evt.preventDefault since hasCustomEventHandling is true whether preventDefault or SC's allowDefault are called. The reason this matters is since the else condition on the rich-text-editor's mouseDown handler calls evt.allowDefault, which, if executed, would make a check for evt.hasCustomEventHandling appear true when the actual behavior I'm testing for has failed.

To further, I went ahead and set up the test to look for a property on the evt.originalEvent called cancelled, which indicates if the code ran through a preventDefault. This value is undefined when the code executes evt.allowDefault.

Anyway, please take a look and let me know if you have additional feedback.

mysterlune commented 9 years ago

@publickeating, any concerns with this PR at this point? Let me know if there's anything you think needs to be done differently, and why. Thanks!

publickeating commented 9 years ago

Sorry for the delayed reply. No more concerns.

Thanks for making those changes. Merging in 3, 2, ...