refactor roundtripping into a single place

and fix isRichTextEnabled to be correctly camelCased everywhere...
This commit is contained in:
Matthew Hodgson 2018-05-20 16:30:39 +01:00
parent aac6866779
commit d799b7e424
4 changed files with 84 additions and 89 deletions

View file

@ -28,8 +28,8 @@ type MessageFormat = 'rich' | 'markdown';
class HistoryItem { class HistoryItem {
// Keeping message for backwards-compatibility // We store history items in their native format to ensure history is accurate
message: string; // and then convert them if our RTE has subsequently changed format.
value: Value; value: Value;
format: MessageFormat = 'rich'; format: MessageFormat = 'rich';
@ -51,32 +51,6 @@ class HistoryItem {
format: this.format format: this.format
}; };
} }
// FIXME: rather than supporting storing history in either format, why don't we pick
// one canonical form?
toValue(outputFormat: MessageFormat): Value {
if (outputFormat === 'markdown') {
if (this.format === 'rich') {
// convert a rich formatted history entry to its MD equivalent
return Plain.deserialize(Md.serialize(this.value));
// return ContentState.createFromText(RichText.stateToMarkdown(contentState));
}
else if (this.format === 'markdown') {
return this.value;
}
} else if (outputFormat === 'rich') {
if (this.format === 'markdown') {
// convert MD formatted string to its rich equivalent.
return Md.deserialize(Plain.serialize(this.value));
// return RichText.htmlToContentState(new Markdown(contentState.getPlainText()).toHTML());
}
else if (this.format === 'rich') {
return this.value;
}
}
console.error("unknown format -> outputFormat conversion");
return this.value;
}
} }
export default class ComposerHistoryManager { export default class ComposerHistoryManager {
@ -110,9 +84,9 @@ export default class ComposerHistoryManager {
sessionStorage.setItem(`${this.prefix}[${this.lastIndex++}]`, JSON.stringify(item.toJSON())); sessionStorage.setItem(`${this.prefix}[${this.lastIndex++}]`, JSON.stringify(item.toJSON()));
} }
getItem(offset: number, format: MessageFormat): ?Value { getItem(offset: number): ?HistoryItem {
this.currentIndex = _clamp(this.currentIndex + offset, 0, this.lastIndex - 1); this.currentIndex = _clamp(this.currentIndex + offset, 0, this.lastIndex - 1);
const item = this.history[this.currentIndex]; const item = this.history[this.currentIndex];
return item ? item.toValue(format) : null; return item;
} }
} }

View file

@ -46,7 +46,7 @@ export default class MessageComposer extends React.Component {
inputState: { inputState: {
marks: [], marks: [],
blockType: null, blockType: null,
isRichtextEnabled: SettingsStore.getValue('MessageComposerInput.isRichTextEnabled'), isRichTextEnabled: SettingsStore.getValue('MessageComposerInput.isRichTextEnabled'),
}, },
showFormatting: SettingsStore.getValue('MessageComposer.showFormatting'), showFormatting: SettingsStore.getValue('MessageComposer.showFormatting'),
isQuoting: Boolean(RoomViewStore.getQuotingEvent()), isQuoting: Boolean(RoomViewStore.getQuotingEvent()),
@ -227,7 +227,7 @@ export default class MessageComposer extends React.Component {
onToggleMarkdownClicked(e) { onToggleMarkdownClicked(e) {
e.preventDefault(); // don't steal focus from the editor! e.preventDefault(); // don't steal focus from the editor!
this.messageComposerInput.enableRichtext(!this.state.inputState.isRichtextEnabled); this.messageComposerInput.enableRichtext(!this.state.inputState.isRichTextEnabled);
} }
render() { render() {
@ -380,10 +380,10 @@ export default class MessageComposer extends React.Component {
<div className="mx_MessageComposer_formatbar" style={this.state.showFormatting ? {} : {display: 'none'}}> <div className="mx_MessageComposer_formatbar" style={this.state.showFormatting ? {} : {display: 'none'}}>
{ formatButtons } { formatButtons }
<div style={{flex: 1}}></div> <div style={{flex: 1}}></div>
<img title={this.state.inputState.isRichtextEnabled ? _t("Turn Markdown on") : _t("Turn Markdown off")} <img title={this.state.inputState.isRichTextEnabled ? _t("Turn Markdown on") : _t("Turn Markdown off")}
onMouseDown={this.onToggleMarkdownClicked} onMouseDown={this.onToggleMarkdownClicked}
className="mx_MessageComposer_formatbar_markdown mx_filterFlipColor" className="mx_MessageComposer_formatbar_markdown mx_filterFlipColor"
src={`img/button-md-${!this.state.inputState.isRichtextEnabled}.png`} /> src={`img/button-md-${!this.state.inputState.isRichTextEnabled}.png`} />
<img title={_t("Hide Text Formatting Toolbar")} <img title={_t("Hide Text Formatting Toolbar")}
onClick={this.onToggleFormattingClicked} onClick={this.onToggleFormattingClicked}
className="mx_MessageComposer_formatbar_cancel mx_filterFlipColor" className="mx_MessageComposer_formatbar_cancel mx_filterFlipColor"

View file

@ -147,17 +147,17 @@ export default class MessageComposerInput extends React.Component {
constructor(props, context) { constructor(props, context) {
super(props, context); super(props, context);
const isRichtextEnabled = SettingsStore.getValue('MessageComposerInput.isRichTextEnabled'); const isRichTextEnabled = SettingsStore.getValue('MessageComposerInput.isRichTextEnabled');
Analytics.setRichtextMode(isRichtextEnabled); Analytics.setRichtextMode(isRichTextEnabled);
this.state = { this.state = {
// whether we're in rich text or markdown mode // whether we're in rich text or markdown mode
isRichtextEnabled, isRichTextEnabled,
// the currently displayed editor state (note: this is always what is modified on input) // the currently displayed editor state (note: this is always what is modified on input)
editorState: this.createEditorState( editorState: this.createEditorState(
isRichtextEnabled, isRichTextEnabled,
MessageComposerStore.getEditorState(this.props.room.roomId), MessageComposerStore.getEditorState(this.props.room.roomId),
), ),
@ -323,7 +323,7 @@ export default class MessageComposerInput extends React.Component {
// this is dirty, but moving all this state to MessageComposer is dirtier // this is dirty, but moving all this state to MessageComposer is dirtier
if (this.props.onInputStateChanged && nextState !== this.state) { if (this.props.onInputStateChanged && nextState !== this.state) {
const state = this.getSelectionInfo(nextState.editorState); const state = this.getSelectionInfo(nextState.editorState);
state.isRichtextEnabled = nextState.isRichtextEnabled; state.isRichTextEnabled = nextState.isRichTextEnabled;
this.props.onInputStateChanged(state); this.props.onInputStateChanged(state);
} }
} }
@ -362,7 +362,7 @@ export default class MessageComposerInput extends React.Component {
const body = escape(payload.text); const body = escape(payload.text);
if (body) { if (body) {
let content = RichText.htmlToContentState(`<blockquote>${body}</blockquote>`); let content = RichText.htmlToContentState(`<blockquote>${body}</blockquote>`);
if (!this.state.isRichtextEnabled) { if (!this.state.isRichTextEnabled) {
content = ContentState.createFromText(RichText.stateToMarkdown(content)); content = ContentState.createFromText(RichText.stateToMarkdown(content));
} }
@ -374,7 +374,7 @@ export default class MessageComposerInput extends React.Component {
startSelection, startSelection,
blockMap); blockMap);
startSelection = SelectionState.createEmpty(contentState.getFirstBlock().getKey()); startSelection = SelectionState.createEmpty(contentState.getFirstBlock().getKey());
if (this.state.isRichtextEnabled) { if (this.state.isRichTextEnabled) {
contentState = Modifier.setBlockType(contentState, startSelection, 'blockquote'); contentState = Modifier.setBlockType(contentState, startSelection, 'blockquote');
} }
let editorState = EditorState.push(this.state.editorState, contentState, 'insert-characters'); let editorState = EditorState.push(this.state.editorState, contentState, 'insert-characters');
@ -582,52 +582,61 @@ export default class MessageComposerInput extends React.Component {
}); });
}; };
enableRichtext(enabled: boolean) { mdToRichEditorState(editorState: Value): Value {
if (enabled === this.state.isRichtextEnabled) return; // for consistency when roundtripping, we could use slate-md-serializer rather than
// commonmark, but then we would lose pills as the MD deserialiser doesn't know about
// them and doesn't have any extensibility hooks.
//
// The code looks like this:
//
// const markdown = this.plainWithMdPills.serialize(editorState);
//
// // weirdly, the Md serializer can't deserialize '' to a valid Value...
// if (markdown !== '') {
// editorState = this.md.deserialize(markdown);
// }
// else {
// editorState = Plain.deserialize('', { defaultBlock: DEFAULT_NODE });
// }
// FIXME: this duplicates similar conversions which happen in the history & store. // so, instead, we use commonmark proper (which is arguably more logical to the user
// they should be factored out. // anyway, as they'll expect the RTE view to match what they'll see in the timeline,
// but the HTML->MD conversion is anyone's guess).
const textWithMdPills = this.plainWithMdPills.serialize(editorState);
const markdown = new Markdown(textWithMdPills);
// HTML deserialize has custom rules to turn matrix.to links into pill objects.
return this.html.deserialize(markdown.toHTML());
}
richToMdEditorState(editorState: Value): Value {
// FIXME: this conversion loses pills (turning them into pure MD links).
// We need to add a pill-aware deserialize method
// to PlainWithPillsSerializer which recognises pills in raw MD and turns them into pills.
return Plain.deserialize(
// FIXME: we compile the MD out of the RTE state using slate-md-serializer
// which doesn't roundtrip symmetrically with commonmark, which we use for
// compiling MD out of the MD editor state above.
this.md.serialize(editorState),
{ defaultBlock: DEFAULT_NODE }
);
}
enableRichtext(enabled: boolean) {
if (enabled === this.state.isRichTextEnabled) return;
let editorState = null; let editorState = null;
if (enabled) { if (enabled) {
// for consistency when roundtripping, we could use slate-md-serializer rather than editorState = this.mdToRichEditorState(this.state.editorState);
// commonmark, but then we would lose pills as the MD deserialiser doesn't know about
// them and doesn't have any extensibility hooks.
//
// The code looks like this:
//
// const markdown = this.plainWithMdPills.serialize(this.state.editorState);
//
// // weirdly, the Md serializer can't deserialize '' to a valid Value...
// if (markdown !== '') {
// editorState = this.md.deserialize(markdown);
// }
// else {
// editorState = Plain.deserialize('', { defaultBlock: DEFAULT_NODE });
// }
// so, instead, we use commonmark proper (which is arguably more logical to the user
// anyway, as they'll expect the RTE view to match what they'll see in the timeline,
// but the HTML->MD conversion is anyone's guess).
const sourceWithPills = this.plainWithMdPills.serialize(this.state.editorState);
const markdown = new Markdown(sourceWithPills);
editorState = this.html.deserialize(markdown.toHTML());
} else { } else {
// let markdown = RichText.stateToMarkdown(this.state.editorState.getCurrentContent()); editorState = this.richToMdEditorState(this.state.editorState);
// value = ContentState.createFromText(markdown);
editorState = Plain.deserialize(
this.md.serialize(this.state.editorState),
{ defaultBlock: DEFAULT_NODE }
);
} }
Analytics.setRichtextMode(enabled); Analytics.setRichtextMode(enabled);
this.setState({ this.setState({
editorState: this.createEditorState(enabled, editorState), editorState: this.createEditorState(enabled, editorState),
isRichtextEnabled: enabled, isRichTextEnabled: enabled,
}, ()=>{ }, ()=>{
this.refs.editor.focus(); this.refs.editor.focus();
}); });
@ -710,7 +719,7 @@ export default class MessageComposerInput extends React.Component {
}; };
onBackspace = (ev: Event, change: Change): Change => { onBackspace = (ev: Event, change: Change): Change => {
if (this.state.isRichtextEnabled) { if (this.state.isRichTextEnabled) {
// let backspace exit lists // let backspace exit lists
const isList = this.hasBlock('list-item'); const isList = this.hasBlock('list-item');
const { editorState } = this.state; const { editorState } = this.state;
@ -740,14 +749,14 @@ export default class MessageComposerInput extends React.Component {
handleKeyCommand = (command: string): boolean => { handleKeyCommand = (command: string): boolean => {
if (command === 'toggle-mode') { if (command === 'toggle-mode') {
this.enableRichtext(!this.state.isRichtextEnabled); this.enableRichtext(!this.state.isRichTextEnabled);
return true; return true;
} }
let newState: ?Value = null; let newState: ?Value = null;
// Draft handles rich text mode commands by default but we need to do it ourselves for Markdown. // Draft handles rich text mode commands by default but we need to do it ourselves for Markdown.
if (this.state.isRichtextEnabled) { if (this.state.isRichTextEnabled) {
const type = command; const type = command;
const { editorState } = this.state; const { editorState } = this.state;
const change = editorState.change(); const change = editorState.change();
@ -913,7 +922,7 @@ export default class MessageComposerInput extends React.Component {
// FIXME: https://github.com/ianstormtaylor/slate/issues/1497 means // FIXME: https://github.com/ianstormtaylor/slate/issues/1497 means
// that we will silently discard nested blocks (e.g. nested lists) :( // that we will silently discard nested blocks (e.g. nested lists) :(
const fragment = this.html.deserialize(transfer.html); const fragment = this.html.deserialize(transfer.html);
if (this.state.isRichtextEnabled) { if (this.state.isRichTextEnabled) {
return change.insertFragment(fragment.document); return change.insertFragment(fragment.document);
} }
else { else {
@ -954,7 +963,7 @@ export default class MessageComposerInput extends React.Component {
if (cmd) { if (cmd) {
if (!cmd.error) { if (!cmd.error) {
this.historyManager.save(editorState, this.state.isRichtextEnabled ? 'rich' : 'markdown'); this.historyManager.save(editorState, this.state.isRichTextEnabled ? 'rich' : 'markdown');
this.setState({ this.setState({
editorState: this.createEditorState(), editorState: this.createEditorState(),
}); });
@ -986,7 +995,7 @@ export default class MessageComposerInput extends React.Component {
const replyingToEv = RoomViewStore.getQuotingEvent(); const replyingToEv = RoomViewStore.getQuotingEvent();
const mustSendHTML = Boolean(replyingToEv); const mustSendHTML = Boolean(replyingToEv);
if (this.state.isRichtextEnabled) { if (this.state.isRichTextEnabled) {
// We should only send HTML if any block is styled or contains inline style // We should only send HTML if any block is styled or contains inline style
let shouldSendHTML = false; let shouldSendHTML = false;
@ -1032,7 +1041,7 @@ export default class MessageComposerInput extends React.Component {
this.historyManager.save( this.historyManager.save(
editorState, editorState,
this.state.isRichtextEnabled ? 'rich' : 'markdown', this.state.isRichTextEnabled ? 'rich' : 'markdown',
); );
if (commandText && commandText.startsWith('/me')) { if (commandText && commandText.startsWith('/me')) {
@ -1119,7 +1128,7 @@ export default class MessageComposerInput extends React.Component {
if (up) { if (up) {
const scrollCorrection = editorNode.scrollTop; const scrollCorrection = editorNode.scrollTop;
const distanceFromTop = cursorRect.top - editorRect.top + scrollCorrection; const distanceFromTop = cursorRect.top - editorRect.top + scrollCorrection;
console.log(`Cursor distance from editor top is ${distanceFromTop}`); //console.log(`Cursor distance from editor top is ${distanceFromTop}`);
if (distanceFromTop < EDGE_THRESHOLD) { if (distanceFromTop < EDGE_THRESHOLD) {
navigateHistory = true; navigateHistory = true;
} }
@ -1128,7 +1137,7 @@ export default class MessageComposerInput extends React.Component {
const scrollCorrection = const scrollCorrection =
editorNode.scrollHeight - editorNode.clientHeight - editorNode.scrollTop; editorNode.scrollHeight - editorNode.clientHeight - editorNode.scrollTop;
const distanceFromBottom = editorRect.bottom - cursorRect.bottom + scrollCorrection; const distanceFromBottom = editorRect.bottom - cursorRect.bottom + scrollCorrection;
console.log(`Cursor distance from editor bottom is ${distanceFromBottom}`); //console.log(`Cursor distance from editor bottom is ${distanceFromBottom}`);
if (distanceFromBottom < EDGE_THRESHOLD) { if (distanceFromBottom < EDGE_THRESHOLD) {
navigateHistory = true; navigateHistory = true;
} }
@ -1168,7 +1177,19 @@ export default class MessageComposerInput extends React.Component {
return; return;
} }
let editorState = this.historyManager.getItem(delta, this.state.isRichtextEnabled ? 'rich' : 'markdown'); let editorState;
const historyItem = this.historyManager.getItem(delta);
if (historyItem) {
if (historyItem.format === 'rich' && !this.state.isRichTextEnabled) {
editorState = this.richToMdEditorState(historyItem.value);
}
else if (historyItem.format === 'markdown' && this.state.isRichTextEnabled) {
editorState = this.mdToRichEditorState(historyItem.value);
}
else {
editorState = historyItem.value;
}
}
// Move selection to the end of the selected history // Move selection to the end of the selected history
const change = editorState.change().collapseToEndOf(editorState.document); const change = editorState.change().collapseToEndOf(editorState.document);
@ -1468,8 +1489,8 @@ export default class MessageComposerInput extends React.Component {
<div className={className}> <div className={className}>
<img className="mx_MessageComposer_input_markdownIndicator mx_filterFlipColor" <img className="mx_MessageComposer_input_markdownIndicator mx_filterFlipColor"
onMouseDown={this.onMarkdownToggleClicked} onMouseDown={this.onMarkdownToggleClicked}
title={this.state.isRichtextEnabled ? _t("Markdown is disabled") : _t("Markdown is enabled")} title={this.state.isRichTextEnabled ? _t("Markdown is disabled") : _t("Markdown is enabled")}
src={`img/button-md-${!this.state.isRichtextEnabled}.png`} /> src={`img/button-md-${!this.state.isRichTextEnabled}.png`} />
<Editor ref="editor" <Editor ref="editor"
dir="auto" dir="auto"
className="mx_MessageComposer_editor" className="mx_MessageComposer_editor"

View file

@ -69,7 +69,7 @@ describe('MessageComposerInput', () => {
'mx_MessageComposer_input_markdownIndicator'); 'mx_MessageComposer_input_markdownIndicator');
ReactTestUtils.Simulate.click(indicator); ReactTestUtils.Simulate.click(indicator);
expect(mci.state.isRichtextEnabled).toEqual(false, 'should have changed mode'); expect(mci.state.isRichTextEnabled).toEqual(false, 'should have changed mode');
done(); done();
}); });
}); });