Skip to content
This repository has been archived by the owner on Apr 2, 2021. It is now read-only.

Image selection #641

Merged
merged 46 commits into from Dec 30, 2014
Merged
Show file tree
Hide file tree
Changes from 43 commits
Commits
Show all changes
46 commits
Select commit Hold shift + click to select a range
e34c686
Select images on mousedown using Rangy
mightyiam Dec 16, 2014
7780bdc
Add test for unlinking image.
mightyiam Dec 16, 2014
2da645b
TridentPre7's _keyup: remove unused assignment
mightyiam Dec 16, 2014
c523284
Image selection test.
mightyiam Dec 16, 2014
0dcea99
Image selection test: assert only one node selected
mightyiam Dec 16, 2014
4a47c58
Use `manipulationFunc` in image selection test
mightyiam Dec 16, 2014
75ce430
Merge branch 'classToggling' into selectImagesWithRangy
mightyiam Dec 17, 2014
5f6e1fb
Make JSHint happy.
mightyiam Dec 17, 2014
1e7dcd8
Merge branch 'master' into selectImagesWithRangy
mightyiam Dec 17, 2014
bf090a9
`editor.getSelectedImage` including api doc and tests
mightyiam Dec 18, 2014
8b918a2
Fix toggleClass
mightyiam Dec 18, 2014
5458c49
Use IMG_SRC in a few places in tests
mightyiam Dec 18, 2014
580db7b
_selectSingleNode
mightyiam Dec 18, 2014
92a1af5
Merge branch 'image-dialog-focus-before-insert' into selectImagesWith…
mightyiam Dec 19, 2014
eab4b0e
Merge branch 'qunit-hide-passed' into selectImagesWithRangy
mightyiam Dec 20, 2014
cf02d83
_selectSingleNode: return statement is supposed to be throw
mightyiam Dec 20, 2014
9a5525f
Fix image dragging and dropping
mightyiam Dec 20, 2014
545c3fb
Docstring for _selectSingleNode
mightyiam Dec 20, 2014
d9a4160
Tests with image selection: skip in PhantomJS
mightyiam Dec 20, 2014
97d1753
Make JSHint happy
mightyiam Dec 20, 2014
d7e9f1b
Some code comments for #641
mightyiam Dec 20, 2014
bb09bdb
Fix code comments for #641
mightyiam Dec 20, 2014
b936084
Changelog for #641.
mightyiam Dec 20, 2014
d581582
Fix Trident 7's docEventQuirks
mightyiam Dec 22, 2014
73a9760
Fix some image related tests
mightyiam Dec 22, 2014
149f898
Remove old image selection test
mightyiam Dec 22, 2014
4388986
Merge branch 'master' into selectImagesWithRangy
mightyiam Dec 22, 2014
1f56d9c
Don't use sinon fake timers in QUnit.
mightyiam Dec 22, 2014
be5fcfd
manipulationTestHelper: async feature
mightyiam Dec 22, 2014
b7d405b
Async image selection test for pre-7 Trident
mightyiam Dec 22, 2014
82715fe
Fix some image related tests for pre-7 Trident
mightyiam Dec 22, 2014
686aac6
Pre-7 trident's image selection test: some comments
mightyiam Dec 22, 2014
37d06d4
Fix some code comment
mightyiam Dec 22, 2014
ec95b71
Test for image selection via dragend in IE
mightyiam Dec 23, 2014
0560db2
Deselect before image selection tests.
mightyiam Dec 23, 2014
6e4b123
Fix an image selection test had extra async: true
mightyiam Dec 23, 2014
306fad5
Added missing `async: true` to test helper example
mightyiam Dec 24, 2014
cb38bce
manipulationTestHelper: more readable async example
mightyiam Dec 24, 2014
99cbd40
manipulationTestHelper: semantic change in returning asyncResumeFunc
mightyiam Dec 24, 2014
e1aed3a
manipulationTestHelper: fail tests that specify async but do not call
mightyiam Dec 24, 2014
6c88b05
Image selection test: more readable.
mightyiam Dec 24, 2014
f25a80a
Unit tests: explain why we do not use fake timers.
mightyiam Dec 24, 2014
c6d0553
Comment on pre-7 Triden'ts `mouseup` on an image.
mightyiam Dec 24, 2014
3c1604f
Better comment for pre-7 Trident image selection
mightyiam Dec 24, 2014
f1921f8
manipulationTestHelper: add comma in docstring
mightyiam Dec 30, 2014
7d90d19
manipulationTestHelper: return asyncResumeFunc only if async
mightyiam Dec 30, 2014
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
8 changes: 5 additions & 3 deletions CHANGELOG.md
Expand Up @@ -20,9 +20,9 @@ WYMeditor.
that is considered unhelpful.
It was previously disabled for a while and it was found to be enabled now.
So it is now disabled again.
* [#648](https://github.com/wymeditor/wymeditor/pull/647)
Similar to the above table editing feature, this feature allows resizing
images and other objects. Similarly, now disabled.
* [#648](https://github.com/wymeditor/wymeditor/pull/647),
[#641](https://github.com/wymeditor/wymeditor/pull/641)
Object resizing (images, primarily) in Firefox and IE has been disabled.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In #648, which is merged already, we disabled object resizing in Gecko.

In this PR, since we can't disable it, we avoid it, in IE.


### Enhancements

Expand All @@ -32,6 +32,8 @@ WYMeditor.
New `editor.doesElementContainSelection` method.
* [#512](https://github.com/wymeditor/wymeditor/pull/512)
New undo/redo methods in the API.
* [#641](https://github.com/wymeditor/wymeditor/pull/641)
New `editor.getSelectedImage()`
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems to work well. Unit tests in all browsers.


### Bug Fixes

Expand Down
6 changes: 6 additions & 0 deletions docs/writing_plugins/api.rst
Expand Up @@ -246,6 +246,12 @@ Example: get the selected root container.

wym.status(wym.mainContainer().tagName);

``getSelectedImage()``
======================

If selection encompasses exactly a single image, returns that image.
Otherwise returns ``false``.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Documentation for new thing!

``canSetCaretBefore(node)``
===========================

Expand Down
74 changes: 62 additions & 12 deletions src/test/unit/manipulation-test-helper.js
Expand Up @@ -64,6 +64,33 @@
* ```
* This example uses the `jquery.browser` plugin
* https://github.com/gabceb/jquery-browser-plugin
* `async`
* Optional; If this is `true` then after the manipulation is performed
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

performed,

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. Done.

* assertions regarding the results are not synchronously executed.
* Instead, a function is returned. Calling this function resumes these
* assertions. This can only be used when a single manipulation cause
* is provided (for example, only `manipulationFunc`).
* For example:
* ```
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winhamwr replaced resume with resumeManipulationTestHelper in the example below. Not runAssertions because it is more than assertions—it is also undo/redo operations. And also, by the time it returns, some assertions were already executed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

* test("Test something asynchronous", function () {
* var wymeditor = jQuery.wymeditors(0),
* somethingAsync,
* resumeManipulationTestHelper;
*
* somethingAsync = wymeditor.somethingAsync;
* wymeditor.somethingAsync = function () {
* somethingAsync.call(wymeditor);
* resumeManipulationTestHelper();
* };
*
* resumeManipulationTestHelper = manipulationTestHelper({
* startHtml: "</p>Foo</p>",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should pass async: true, here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. Fixed.

* async: true,
* manipulationClickSelector: ".asyncActionButton",
* expectedResultHtml: "</p>Bar</p>"
* });
* });
* ```
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In pre–7 Trident (IE <= 10) image selection via mouseup works asynchronously.

This helps test that and perhaps future things.

*
* `manipulationFunc` and `manipulationClickSelector` are not exclusive
* of each other. The procedure will be performed once for each of them.
Expand All @@ -72,7 +99,8 @@
function manipulationTestHelper(a) {
var executions = [],
wymeditor,
EXECUTE;
EXECUTE,
asyncResumeFunc;

if (skipThisTest() === true) {
return;
Expand Down Expand Up @@ -100,29 +128,51 @@ function manipulationTestHelper(a) {
manipulateAndAssert(EXECUTE.NO_MANIPULATION);
}

if (
a.async === true &&
executions.length > 1
) {
throw "The `async` option is only allowed with one manipulation cause";
}

while (executions.length > 0) {
manipulateAndAssert(executions.pop());
asyncResumeFunc = manipulateAndAssert(executions.pop());
}

return asyncResumeFunc;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@winhamwr you're right. This is much cleaner. I thought about writing it like this, at first, but setting it inside a while loop statement didn't seem reasonable, until now that I realized that in the case when a.async === true, that loop will execute once.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While this code functions correctly, it reads poorly. There's no reason to return here if we're not using async (even though the caller should just ignore the return result). Let's put it inside an if (a.async === true) block.


function manipulateAndAssert(manipulationCause) {

initialize();
assertStartHtml();
resetHistory();

performManipulation(manipulationCause);
assertResultHtml();
additionalAssertions();

if (a.testUndoRedo !== true) {
return;
// Expectancy incremented here in order to fail tests that specify
// `async` but do not call the `asyncResumeFunc`.
expect(expect() + 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Here's the fix for the case where a test specifies async: true but does not call the returned function.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Clever fix!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

if (a.async === true) {
return assertResultUndoAndAdditional;
} else {
assertResultUndoAndAdditional();
}
wymeditor.undoRedo.undo();
assertStartHtml("Back to start HTML after undo");

wymeditor.undoRedo.redo();
assertResultHtml("Back to result HTML after redo");
additionalAssertions();
function assertResultUndoAndAdditional() {
// Return expectancy to real value.
expect(expect() - 1);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See comment above. This is where it is returned to normal.

assertResultHtml();
additionalAssertions();

if (a.testUndoRedo !== true) {
return;
}
wymeditor.undoRedo.undo();
assertStartHtml("Back to start HTML after undo");

wymeditor.undoRedo.redo();
assertResultHtml("Back to result HTML after redo");
additionalAssertions();
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the code that runs after performManipulation is crammed into this function, assertResultUndoAndAdditional.

If this is an asynchronous test, resume is set to this function and returned from manipulationTestHelper. Else, it is called now.


function initialize() {
if (typeof a.startHtml === 'string') {
Expand Down
10 changes: 8 additions & 2 deletions src/test/unit/specific_feature_tests/class_toggling.js
Expand Up @@ -2,6 +2,8 @@
prepareUnitTestModule,
test,
manipulationTestHelper,
inPhantomjs,
SKIP_THIS_TEST,
IMG_SRC
*/
"use strict";
Expand Down Expand Up @@ -154,7 +156,8 @@ test("Adds className to image", function () {
, "</p>"
].join(""),
prepareFunc: function (wymeditor) {
wymeditor.$body().find("img").mousedown();
var img = wymeditor.$body().find("img")[0];
wymeditor._selectSingleNode(img);
},
manipulationFunc: function (wymeditor) {
wymeditor.toggleClass("fancy", "img");
Expand All @@ -165,6 +168,9 @@ test("Adds className to image", function () {
, "Foo"
, "<img alt=\"foo\" class=\"fancy\" src=\"" + IMG_SRC + "\" />"
, "</p>"
].join("")
].join(""),
skipFunc: function () {
return inPhantomjs ? SKIP_THIS_TEST : null;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This and other tests that involve image selection are skipped in PhantomJS. Because I don't have time for this.

});
});