-
Notifications
You must be signed in to change notification settings - Fork 2k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Editor: Add support for editing embeds inside a post
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
Showing
7 changed files
with
297 additions
and
0 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
1 change: 1 addition & 0 deletions
1
client/components/tinymce/plugins/wpcom-view/views/embed/embed-dialog/README.md
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 |
131 changes: 131 additions & 0 deletions
131
client/components/tinymce/plugins/wpcom-view/views/embed/embed-dialog/index.jsx
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import PropTypes from 'prop-types'; | ||
import React from 'react'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import Button from 'components/button'; | ||
import Dialog from 'components/dialog'; | ||
import FormTextInput from 'components/forms/form-text-input'; | ||
import { localize } from 'i18n-calypso'; | ||
import { identity } from 'lodash'; | ||
|
||
// todo | ||
// lint branch before commit | ||
// add jsdoc to all functions | ||
|
||
export class EmbedDialog extends React.Component { | ||
static propTypes = { | ||
embedUrl: PropTypes.string, | ||
isVisible: PropTypes.bool, | ||
|
||
// Event handlers | ||
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 | ||
|
||
// Inherited | ||
translate: PropTypes.func, | ||
// todo make required and move identity to unit tests unless jeff comments on remove-button PR | ||
}; | ||
|
||
static defaultProps = { | ||
embedUrl: '', | ||
isVisible: false, | ||
translate: identity, | ||
}; | ||
|
||
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? | ||
// if remove, remove from mockChangeEvent too | ||
}; | ||
|
||
onCancel = () => { | ||
this.setState( { isVisible: false } ); | ||
}; | ||
|
||
onUpdate = () => { | ||
this.props.onInsert( this.state.embedUrl ); | ||
this.setState( { isVisible: false } ); | ||
}; | ||
|
||
render() { | ||
const { translate } = this.props; | ||
|
||
return ( | ||
<Dialog | ||
className="embed-dialog" | ||
additionalClassNames="embed-dialog__modal" | ||
isVisible={ this.state.isVisible } | ||
onClose={ this.onCancel } | ||
buttons={ [ | ||
<Button onClick={ this.onCancel }> | ||
{ translate( 'Cancel' ) } | ||
</Button>, | ||
<Button primary onClick={ this.onUpdate }> | ||
{ translate( 'Update' ) } | ||
</Button> | ||
] }> | ||
<h3 className="embed-dialog__title"> | ||
{ translate( '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 | ||
... | ||
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 | ||
test localized strings in other locale | ||
*/} | ||
</Dialog> | ||
); | ||
} | ||
} | ||
|
||
export default localize( EmbedDialog ); |
26 changes: 26 additions & 0 deletions
26
client/components/tinymce/plugins/wpcom-view/views/embed/embed-dialog/style.scss
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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 */ | ||
|
79 changes: 79 additions & 0 deletions
79
client/components/tinymce/plugins/wpcom-view/views/embed/embed-dialog/test/index.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,79 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import React from 'react'; | ||
import FormTextInput from 'components/forms/form-text-input'; | ||
import { assert } from 'chai'; | ||
import { shallow } from 'enzyme'; | ||
import { noop } from 'lodash'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import { EmbedDialog } from '../index'; | ||
|
||
describe( 'EmbedDialog', function() { | ||
it( "should update the input field's value when input changes", function() { | ||
const originalUrl = 'https://www.youtube.com/watch?v=ghrL82cc-ss', | ||
newUrl = 'https://videopress.com/v/DNgJlco8', | ||
wrapper = shallow( <EmbedDialog embedUrl={ originalUrl } onInsert={ noop } /> ), | ||
mockChangeEvent = { | ||
target: { | ||
value: newUrl, | ||
focus: noop, | ||
} | ||
}; | ||
let inputField = wrapper.find( FormTextInput ).get( 0 ); | ||
|
||
assert.strictEqual( inputField.props.defaultValue, originalUrl ); | ||
|
||
wrapper.find( FormTextInput ).simulate( 'change', mockChangeEvent ); | ||
inputField = wrapper.find( FormTextInput ).get( 0 ); | ||
assert.strictEqual( inputField.props.defaultValue, newUrl ); | ||
} ); | ||
|
||
it( 'should return the new url to onInsert when updating', function() { | ||
const originalUrl = 'https://www.youtube.com/watch?v=R54QEvTyqO4', | ||
newUrl = 'https://videopress.com/v/x4IYthy7', | ||
mockChangeEvent = { | ||
target: { | ||
value: newUrl, | ||
focus: noop, | ||
} | ||
}; | ||
let currentUrl = originalUrl; | ||
const onInsert = ( url ) => { | ||
currentUrl = url; | ||
}; | ||
const wrapper = shallow( <EmbedDialog embedUrl={ originalUrl } onInsert={ onInsert } /> ); | ||
|
||
wrapper.find( FormTextInput ).simulate( 'change', mockChangeEvent ); | ||
wrapper.instance().onUpdate(); | ||
assert.strictEqual( currentUrl, newUrl ); | ||
} ); | ||
|
||
it( 'should not return the new url to onInsert when canceling', function() { | ||
const originalUrl = 'https://www.youtube.com/watch?v=JkOIhs2mHpc', | ||
newUrl = 'https://videopress.com/v/GtWYbzhZ', | ||
mockChangeEvent = { | ||
target: { | ||
value: newUrl, | ||
focus: noop, | ||
} | ||
}; | ||
let currentUrl = originalUrl; | ||
const onInsert = ( url ) => { | ||
currentUrl = url; | ||
}; | ||
const wrapper = shallow( <EmbedDialog embedUrl={ originalUrl } onInsert={ onInsert } /> ); | ||
|
||
wrapper.find( FormTextInput ).simulate( 'change', mockChangeEvent ); | ||
wrapper.instance().onCancel(); | ||
assert.strictEqual( currentUrl, originalUrl ); | ||
} ); | ||
|
||
// todo | ||
// 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) | ||
} ); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters