New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dialogs overhaul #639

Merged
merged 92 commits into from Jan 22, 2015

Conversation

Projects
None yet
2 participants
@mightyiam
Member

mightyiam commented Dec 13, 2014

No description provided.

},
manipulationFunc: function () {
jQuery(args.clickSelector).click();
},

This comment has been minimized.

@mightyiam

mightyiam Dec 16, 2014

Member

In #624 I'll also add an option to this helper that tests opening of dialogs via key combo.

@mightyiam mightyiam referenced this pull request Dec 18, 2014

Merged

Image selection #641

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

This comment has been minimized.

@mightyiam
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.

This comment has been minimized.

@mightyiam
@@ -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()`

This comment has been minimized.

@mightyiam
@@ -65,6 +65,7 @@
"specific_feature_tests/images.js",
"specific_feature_tests/class_toggling.js",
"specific_feature_tests/links.js",
"specific_feature_tests/dialogs.js",

This comment has been minimized.

@mightyiam

mightyiam Dec 23, 2014

Member

Add new tests file to suite.

@@ -64,6 +64,32 @@
* ```

This comment has been minimized.

@mightyiam

mightyiam Dec 23, 2014

Member

All from #641.

@@ -2,6 +2,8 @@
prepareUnitTestModule,

This comment has been minimized.

@mightyiam

mightyiam Dec 23, 2014

Member

All from #641.

"use strict";
/*
* This file contains tests for dialogs.

This comment has been minimized.

@mightyiam

mightyiam Dec 23, 2014

Member

New! Make sure that dialogs open only when they are useful.

// This unwraps the original functions from the spies after each test.
while (sinonSpiesToRestore.length > 0) {
sinonSpiesToRestore.pop().restore();
}

This comment has been minimized.

@mightyiam

mightyiam Dec 23, 2014

Member

I loved learning more about Sinon.

@@ -460,30 +460,6 @@ Example: switch the root container to Heading 1.
wym.mainContainer('H1');
``wrap(left, right)``

This comment has been minimized.

@mightyiam

mightyiam Jan 15, 2015

Member

These two removed.

);
};
WYMeditor.editor.prototype.unwrap = function () {

This comment has been minimized.

@mightyiam

mightyiam Jan 15, 2015

Member

That's a wrap for unwrap.

Removed documentation and added changelog entry.

options.preInitDialog(wym, wDialog);
}
// Dialog-specific initialization

This comment has been minimized.

@mightyiam

mightyiam Jan 15, 2015

Member

@winhamwr I did scrape existing image and link autopopulation from here and put it in each dialog object's initialize property function.

Autopopulation seems too specific so I made it a general initialize.

wDialog.close();
});
// Post-init function

This comment has been minimized.

@mightyiam

mightyiam Jan 15, 2015

Member

Thanks for detecting the typo. Fixed.

@@ -364,21 +364,10 @@ Undo
undo an action.
Redo
redo an action.
CreateLink
open the link dialog and create/update a link on the selection.

This comment has been minimized.

@mightyiam

mightyiam Jan 15, 2015

Member

exec command can no longer be used to open dialogs. Changelog entry added.

@@ -712,6 +664,53 @@ Example:
var $buttons = wym.get$Buttons();
*******
Dialogs

This comment has been minimized.

@mightyiam

mightyiam Jan 17, 2015

Member

API sweetness

@@ -665,19 +630,6 @@ Example:
wym.status("This is the status bar.");
``dialog(sType)``

This comment has been minimized.

@mightyiam

mightyiam Jan 17, 2015

Member

Moved this to its own category.

* Changes in `wym.dialog` method. Existing custom dialogs should still work,
but it is recommended that existing custom dialogs be rewritten using the
new `wym.dialog` API (see API documentation) or at least that they be
tested.

This comment has been minimized.

@mightyiam

mightyiam Jan 20, 2015

Member

@winhamwr please review these two changelog entries.

This comment has been minimized.

@winhamwr

winhamwr Jan 22, 2015

Member

Should the wym.wrap and wym.unwrap removal be in backward incompatible changes?

This comment has been minimized.

@mightyiam

mightyiam Jan 22, 2015

Member

It seems obvious to me that removal of public API must be mentioned in backward incompatible changes.

This comment has been minimized.

@mightyiam

mightyiam Jan 22, 2015

Member

I've merged. If you feel different than me, please share @winhamwr.

@mightyiam mightyiam referenced this pull request Jan 20, 2015

Merged

Combokeys #624

Dialogs
*******
``dialog(dialogObject)``

This comment has been minimized.

@mightyiam

mightyiam Jan 20, 2015

Member

@winhamwr I've changed the API so that the official way is to expect an object. This seems the most straightforward way to me.

This comment has been minimized.

@mightyiam

mightyiam Jan 20, 2015

Member

I've altered the documentation here to correspond to that change, as well. Please review.

doc,
selectedContainer,
options = wym._options;

This comment has been minimized.

@mightyiam

mightyiam Jan 20, 2015

Member

Some changes below, to accept a dialog object while maintaining backwards compatibility for accepting a string.

// The following would also work, but is deprecated:
// wym.dialog(dialogName);
wym.dialog(dialog);

This comment has been minimized.

@mightyiam

mightyiam Jan 20, 2015

Member

@winhamwr provide a dialog object.

Optional. Handles a submit button press in the dialog. Is called with
the editor instance as ``this``. Receives a single argument-the dialog
window.

This comment has been minimized.

@winhamwr

winhamwr Jan 22, 2015

Member

Hooray APIs!

This comment has been minimized.

@mightyiam
@winhamwr

This comment has been minimized.

Member

winhamwr commented Jan 22, 2015

Other than maybe needing to remove the examples links and the backwards incompatible changes entry, this looks ready to merge!

mightyiam added a commit that referenced this pull request Jan 22, 2015

@mightyiam mightyiam merged commit f798cf5 into master Jan 22, 2015

1 check was pending

continuous-integration/travis-ci The Travis CI build is in progress
Details

@mightyiam mightyiam deleted the dialogWindowTests branch Jan 22, 2015

@@ -1,89 +0,0 @@
<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">

This comment has been minimized.

@mightyiam

mightyiam Jan 22, 2015

Member

@winhamwr I've removed this just before merging now.

This would be a bad example as it is.

The API seems well documented, so an example seems not so important.

This comment has been minimized.

@winhamwr

winhamwr Jan 22, 2015

Member

I think maybe you underestimate the value of examples in learning. I think it would be well-worth timeboxing 15 minutes to update this example to use the new API and to then link to the example from the documentation. Examples are basically the only way some people learn :)

Thoughts?

@mightyiam

This comment has been minimized.

Member

mightyiam commented Jan 22, 2015

Awesome.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment