Skip to content

Commit

Permalink
Editor: Add support for editing embeds inside a post
Browse files Browse the repository at this point in the history
This adds a dialog to edit the embed URL, but doesn't add a preview of the new URL, so it doesn't fully implement [the design in #1729](https://user-images.githubusercontent.com/191598/28643536-23f2e318-7224-11e7-8fd1-9a889d53b594.png). It's a step in that direction, though, and a future PR will add the preview.

See #1729
  • Loading branch information
iandunn committed Sep 21, 2017
1 parent fb33db9 commit 8084d6b
Show file tree
Hide file tree
Showing 7 changed files with 250 additions and 0 deletions.
59 changes: 59 additions & 0 deletions client/components/tinymce/plugins/wpcom-view/plugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ var tinymce = require( 'tinymce/tinymce' ),
import views from './views';
import { renderWithReduxStore } from 'lib/react-helpers';
import { getSelectedSiteId } from 'state/ui/selectors';
import EmbedDialog from 'components/tinymce/plugins/wpcom-view/views/embed/embed-dialog';

/**
* WordPress View plugin.
Expand Down Expand Up @@ -825,6 +826,64 @@ function wpview( editor ) {
}
});

editor.addCommand( 'embedEditLink', content => {
const node = editor.selection.getNode();

//console.log('node',node);
//console.log( window.foo === node );

// maybe it changes b/c it's a node that tinemyce dynmaically creates a destroys? need to create our own permenant element outside of the editor instead?

// it is changing. maybe need to select it some other way. see what other places in calypso use.
// or maybe it inevitable b/c tinymce replaces things? in that case, maybe just need to destroy the old one

// this is creating an infinite number instead of creating 1 and reusing
// that's what contact-form and simple-payments do, though?
// maybe assign the React.createLement statement to a variable, then pass it to renderwithreduxstore?
// jeff said: Maybe the `node` value is changing or something

// wp_help creates an empty dom element when editor loads and reuses it

/*
let node;
if ( window.foo ) {
node = window.foo;
console.log('reused foo');
} else {
window.foo = node = editor.selection.getNode();
console.log('new foo')
}
console.log( 'foo', window.foo );
*/


const store = editor.getParam( 'redux_store' );
const siteId = getSelectedSiteId( store.getState() );

ReactDom.render(
React.createElement( EmbedDialog, {
embedUrl: content,
isVisible: true,
siteId: siteId,
onInsert: function( embedUrl ) {
console.log('oninsert', embedUrl );
// ugh, somewhere along the line this broke. now it's just inserting it into the anchor text at the begining of the editor content.
// maybe it always did that? need to reproduce reliably, might have to click in and out of embed focus, open/close, etc. or maybe it hapepns every time now.
// caused by calling this.embedViewManager.updateSite( this.props.siteId ); -- wtf, why?
// huh, if you do it several times in a row, it will work some times but not others. never seen it work on first load though
editor.execCommand( 'mceInsertContent', false, embedUrl );
editor.focus(); // otherwise the user has to click outside the element and then back on it again if they want the edit/remove dialog to re-appear -- todo explain better
// do on cancel too
},
} ),
node
);


// bugs with ctrl-z after editing?
} );

editor.addButton( 'wp_view_edit', {
tooltip: i18n.translate( 'Edit', { context: 'verb' } ),
icon: 'dashicon dashicons-edit',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
give high-level overview of what it is and how to use it
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
/**
* External dependencies
*/
import React, { Component } from 'react';
import PropTypes from 'prop-types';

/**
* Internal dependencies
*/
import Dialog from 'components/dialog';
import Button from 'components/button';
import FormTextInput from 'components/forms/form-text-input';

// lint branch before commit
// add jsdoc to all functions

class EmbedDialog extends Component {
static propTypes = {
embedUrl: PropTypes.string,
isVisible: PropTypes.bool,
onInsert: PropTypes.func.isRequired,
// change to not required and set default to noop? or go the other direction and make embedurl and siteid required too?
// probably rename to something more generic, b/c this could be used outside of tinymce context
};

static defaultProps = {
embedUrl: '',
isVisible: false,
};

state = {
embedUrl: this.props.embedUrl,
isVisible: this.props.isVisible,
};

onChangeEmbedUrl = ( event ) => {
//console.log( 'onchange - focus:', document.activeElement );

this.setState( {
embedUrl: event.target.value,
// todo really need to do this manually? seems like something react could do automatically
} );

// the debounce works, but the focus is jumping back to the start of the editor, probably related to the onInsert problem.
// maybe it's because the embedview inside the editor is also refreshing? how to stop that to test if that fixes problem?

event.target.focus(); //todo hack to avoid focus stealiing. is it needed in this pr, or just the preview branch?
};

onCancel = () => {
this.setState( { isVisible: false } );
};

onUpdate = () => {
this.props.onInsert( this.state.embedUrl );
this.setState( { isVisible: false } );
};

render() {
return (
<Dialog
className="embed-dialog"
additionalClassNames="embed-dialog__modal"
isVisible={ this.state.isVisible }
onClose={ this.onCancel }
buttons={ [
<Button onClick={ this.onCancel }>
Cancel
</Button>,
<Button primary onClick={ this.onUpdate }>
Update
</Button>
] }>
<h3 className="embed-dialog__title">Embed URL</h3>

<FormTextInput
className="embed-dialog__url"
defaultValue={ this.state.embedUrl }
onChange={ this.onChangeEmbedUrl }
/>

{/*
todo now input field won't update w/ new state. er, maybe it does, but problem is that focus is often stolen?
// working now, maybe it was the css change? make sure still working in a few
hitting enter in input field should update, hitting escape should cancel
exception thrown when change it twice in a row. - only in FF
maybe related to needing to debounce?
Warning: unmountComponentAtNode(): The node you're attempting to unmount was rendered by another copy of React.
wrapConsole/<
app:///./client/components/webpack-build-monitor/index.jsx:174:3
printWarning
app:///./node_modules/fbjs/lib/warning.js:35:7
warning
app:///./node_modules/fbjs/lib/warning.js:59:7
unmountComponentAtNode
app:///./node_modules/react-dom/lib/ReactMount.js:443:15
wpview/</<
>>> app:///./client/components/tinymce/plugins/wpcom-view/plugin.js:287:5
...
test videos
https://www.youtube.com/watch?v=R54QEvTyqO4
https://www.youtube.com/watch?v=ghrL82cc-ss
https://www.youtube.com/watch?v=JkOIhs2mHpc
iCvmsMzlF7o&
get some others video platforms, and maybe some non-video ones too
also verify that only whitelisted embeds will work, and that all other user input is discarded to avoid security issues
make sure there aren't any execution sinks, etc
localize strings and test in other locale
*/}
</Dialog>
);
}
}

export default EmbedDialog;
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
.dialog.card.embed-dialog__modal {
width: 80%;
max-width: 600px;
/* todo better way to set this than hardcode pixels? will need to be 690px speciailly when add preview, though, right? */
}

.embed-dialog__modal .dialog__action-buttons:before {
background: none;
}

.embed-dialog__title {
color: #4b6476; /* don't see an existing sass color for this */
font-weight: bold; /* too much, need a semi-bold or different font or something, but that wouldn't be the proper way */
}

input[type="text"].embed-dialog__url {
margin-top: 1em;
}

/*
cancel
bg color f3f6f8? is tehre already a button class for that? if not, prob just use whatever already exists, and let shaun ask for revision if he really intended different
*/

/* look at responsnive. do mobile-first */

Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
/**
* External dependencies
*/
import { expect } from 'chai';
import { shallow } from 'enzyme';
import { noop } from 'lodash';
import React from 'react';

/**
* Internal dependencies
*/
import EmbedDialog from '../index';

describe( 'EmbedDialog', () => {
it( 'should render when passed an onInsert callback', () => {
const wrapper = shallow( <EmbedDialog onInsert={ noop } /> );

expect( wrapper.find( '.embed-dialog' ).length ).to.equal( 1 );

// can probably get rid of this once add the others
} );

// update the state when new input given

// return the new url to onInsert after updated

// leave the old url untouched when canceld

// not let or cause focus to get stolen

// should test valid vs invalid embed urls? maybe only once get to preview PR

// update the preview when new url given (other PR)
} );
Original file line number Diff line number Diff line change
Expand Up @@ -34,11 +34,13 @@ export default class EmbedViewManager extends EventEmitter {
if ( ! this.hasOwnProperty( 'siteId' ) ) {
// First update (after adding initial listener) should trigger a
// fetch, but not emit a change event
//console.log('updatesite first update');
this.siteId = siteId;
this.fetchSiteEmbeds();
} else if ( this.siteId !== siteId ) {
// Subsequent updates should neither emit a change nor trigger a
// fetch unless the site has changed
//console.log('udpatesite onchange');
this.siteId = siteId;
this.onChange();
}
Expand Down Expand Up @@ -135,4 +137,8 @@ export default class EmbedViewManager extends EventEmitter {
getComponent() {
return EmbedView;
}

edit( editor, content ) {
editor.execCommand( 'embedEditLink', content );
}
}
1 change: 1 addition & 0 deletions client/components/tinymce/style.scss
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
@import 'plugins/insert-menu/style';
@import 'plugins/wpcom-help/style';
@import 'plugins/wpcom-charmap/style';
@import 'plugins/wpcom-view/views/embed/embed-dialog/style.scss';
@import 'plugins/contact-form/style';
@import 'plugins/mentions/style';
@import 'plugins/simple-payments/style';
Expand Down

0 comments on commit 8084d6b

Please sign in to comment.