ContentTools icon indicating copy to clipboard operation
ContentTools copied to clipboard

Add prev/next navigation support for text, images, video. Fix undo/redo toolbox issue.

Open cubiclesoft opened this issue 6 years ago • 5 comments

Unfortunately, due to a messed up master (see my other open pull request), this branch also includes the earlier code for fixing touch support for CT toolbox. (I'll do better branch-merging in the future.)

The changes here add keyboard navigation support for images and video elements to other elements. I also fixed a bug that I ran into where undo and redo via keyboard were not functioning as expected (case statements in Javascript don't accept expressions).

Partially fixes some of the issues in #487.

Related pull request: https://github.com/GetmeUK/ContentEdit/pull/25

cubiclesoft avatar Mar 20 '18 14:03 cubiclesoft

@cubiclesoft Thanks for this (and the PR on ContentEdit) I'll be merging them in this weekend (awesome work) :+1:

Can I check with this PR, if I merge in this code with the touch support is there anything I should be aware of around touch support, e.g does touch support change the functionality of the editor beyond simple adding the ability to use touch events?

anthonyjb avatar Mar 22 '18 09:03 anthonyjb

https://github.com/GetmeUK/ContentTools/pull/439

To get touch support working for toolbox, I reverted back to master from mobile_ready and added support for touch events in. So it's definitely a pristine commit against master. There's a demo page with just #439 if you want to test/verify:

https://cubiclesoft.com/Unrelated/ContentToolsDragTest/sandbox/

Looking back at the commit, the only obvious significant change that I see is the addition of ev.preventDefault(), which only affects touchstart (i.e. prevent browser scrolling for touch events while dragging the toolbox) whereas there is no default browser behavior for mousedown events (that I'm aware of).

cubiclesoft avatar Mar 23 '18 13:03 cubiclesoft

Just thought of one minor bug: When switching regions, the 'next-region' and 'previous-region' triggers will still jump to 'content' it finds instead of the navigable. That is, _handleNextRegionTransition() and _handlePreviousRegionTransition() probably need a minor adjustment or two to clean up transitioning regions if there is non-content at the start or end of a region (or if the region only contains non-content elements). This bug only affects those who use multiple regions on a page but it's no worse than the existing behavior.

cubiclesoft avatar Mar 23 '18 16:03 cubiclesoft

Latest commit fixes next/previous region navigation.

cubiclesoft avatar Mar 24 '18 02:03 cubiclesoft

Thanks for the update :+1:

anthonyjb avatar Mar 27 '18 21:03 anthonyjb