GetmeUK / ContentTools

A JS library for building WYSIWYG editors for HTML content.
http://getcontenttools.com
MIT License
3.95k stars 395 forks source link

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

Open cubiclesoft opened 6 years ago

cubiclesoft commented 6 years ago

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

anthonyjb commented 6 years ago

@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?

cubiclesoft commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

Latest commit fixes next/previous region navigation.

anthonyjb commented 6 years ago

Thanks for the update :+1: