simplajs / simpla-img

An editable image you can update inline, built on Simpla
https://www.simplajs.org
MIT License
6 stars 3 forks source link

Fake focus on image when editing #65

Closed madeleineostoja closed 7 years ago

madeleineostoja commented 7 years ago

Fire focusin on the originating img element when the global editor singleton opens, to maintain proper focus heirachy for any parent elements of simpla-img. This is because the editor is meant to look like it originates from the img element, but it's actually attached to the root of the document.

See https://github.com/SimplaElements/simpla-link/issues/3 for example of this issue.

madeleineostoja commented 7 years ago

Note this won't fix simpla-link as of https://github.com/SimplaElements/simpla-link/commit/14816b5c6df6c2871deedd81dee1016c6e2dbda7, but it's still a start. What it should really do is properly keep the image in focus, but then we lose the 'close editor on blur' functionality, which probably wouldn't be worth it.

bedeoverend commented 7 years ago

I don't think we should add this unless it's definitely involved in getting it to work with simpla-link, otherwise we're making it fire something for not a lot of reason, which could end up biting us in the ass I think.

I'll have more of a think on how to get it to work properly with link - if it is involved in getting it to work, then definitely happy to merge. Ideally we'd have some ultra solid way of doing this, but I can't think how...

bedeoverend commented 7 years ago

Perhaps we could just try killing the focusout and blur events on simpla-img? i.e.

listeners: {
  'focusout': '_stopEvent',
  'blur': '_stopEvent'
},

_stopEvent(event) {
  if (this.willFocusOnEditor(event)) {
    event.stopImmediatePropagation();
  }
}

That's just psuedo code, you'd obviously have to make sure it doesn't screw with any of the current blur handling. Worth a shot though I reckon.

madeleineostoja commented 7 years ago

Regardless of simpla-link, trying to retain 'focus' on the originating simpla-img is a more robust pattern, since it's designed to look like the image 'becomes' the editor, so any parents that might rely on focus get a helping hand.

Either way though yep that's a much better idea, just squashing the focusout event (wouldn't need to squash blur for simpla-link, since it doesn't bubble, but I guess it makes sense in general for keeping an image appearing to be focussed).

I'll update this PR and see what you think

madeleineostoja commented 7 years ago

@bedeoverend can you LGTM? This now properly makes an image focuseable when editable, and tries to fake focus/blur behavior on the image as best it can when editing.

Re: simpla-link, this works now except for when you upload an image, since opening the file picker causes active to go momentarily false. Which is a separate problem that should be fixed regardless, as it'd impact anything that relies on active. But for simpla-link that's a perfectly fine bug/edge-case.