Skip to content

Editable: Adding an inline link control to the format toolbar #524

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.

Sign up for GitHub

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

Merged
youknowriad merged 6 commits into from
May 2, 2017
Merged

Editable: Adding an inline link control to the format toolbar #524

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
28b7e06
Editable: Adding an inline link control to the format toolbar
youknowriad Apr 27, 2017
d6a006c
Format Toolbar: Small style changes
youknowriad Apr 27, 2017
609f2a5
Link Control: Adding the view link state
youknowriad Apr 28, 2017
328b212
Link control: Trigger the edit state when inserting an new link
youknowriad Apr 28, 2017
31a5778
Format Controls: Fixing the empty link insertion
youknowriad May 1, 2017
910d2dc
Format Controls: Leverage the Toolbar component
youknowriad May 1, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Format Toolbar: Small style changes
  • Loading branch information
@youknowriad
youknowriad committed May 2, 2017
commit d6a006cc2c61eef59f8950be0b0a9b1c2c91d922
10 changes: 4 additions & 6 deletions blocks/components/editable/format-toolbar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,11 @@ import classNames from 'classnames';
/**
* Internal dependencies
*/
import './style.scss';

// TODO: We mustn't import by relative path traversing from blocks to editor
// as we're doing here; instead, we should consider a common components path.
import IconButton from '../../../editor/components/icon-button';

const formattingControls = [
const FORMATTING_CONTROLS = [
{
icon: 'editor-bold',
title: wp.i18n.__( 'Bold' ),
Expand Down Expand Up @@ -85,12 +83,12 @@ class FormatToolbar extends wp.element.Component {
const { formats, focusPosition } = this.props;
const linkStyle = focusPosition
? { position: 'absolute', ...focusPosition }
: {};
: null;

return (
<div>
<ul className="editable-format-toolbar editor-toolbar">
{ formattingControls.map( ( control, index ) => (
{ FORMATTING_CONTROLS.map( ( control, index ) => (
<IconButton
key={ index }
icon={ control.icon }
Expand All @@ -116,7 +114,7 @@ class FormatToolbar extends wp.element.Component {
style={ linkStyle }
onSubmit={ this.submitLink }>
<input type="url" value={ this.state.linkValue } onChange={ this.updateLinkValue } />
<IconButton icon="editor-break" onClick={ this.submitLink } />
<IconButton icon="editor-break" type="submit" />
<IconButton icon="trash" onClick={ this.dropLink } />
</form>
}
Expand Down
5 changes: 2 additions & 3 deletions blocks/components/editable/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import classnames from 'classnames';
import { last, isEqual, capitalize, omitBy } from 'lodash';
import { last, isEqual, capitalize, omitBy, forEach } from 'lodash';
import { nodeListToReact } from 'dom-react';
import { Fill } from 'react-slot-fill';
import 'element-closest';
Expand Down Expand Up @@ -319,8 +319,7 @@ export default class Editable extends wp.element.Component {
changeFormats( formats ) {
this.editor.focus();

Object.keys( formats ).forEach( format => {
const formatValue = formats[ format ];
forEach( formats, ( formatValue, format ) => {
if ( format === 'link' ) {
Copy link
Member

@aduth aduth Apr 27, 2017

Choose a reason for hiding this comment

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

Based on the distinctly unique treatment between links and other formatting, I'm wondering if they ought to be handled separately. Maybe not; I do like the single onChange prop applied to the toolbar component.

if ( formatValue !== undefined ) {
Copy link
Member

@aduth aduth Apr 27, 2017

Choose a reason for hiding this comment

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

To this and the above TODO, what's the use case for an empty link? Should we just check general truthiness here, including treating an empty string as an unlink intent?

Copy link
Contributor Author

@youknowriad youknowriad Apr 27, 2017

Choose a reason for hiding this comment

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

The use-case is showing the link modal at the right position when we toggle the link control in the toolbar.

To show the link modal we could rely on a state boolean but we need a dom element around our selection to compute the right position for the link modal. So my idea is to insert an empty link when we hit the link control.

this.editor.execCommand( 'mceInsertLink', true, formatValue );
Expand Down
8 changes: 4 additions & 4 deletions languages/gutenberg.pot
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,19 +3,19 @@ msgstr ""
"Content-Type: text/plain; charset=utf-8\n"
"X-Generator: babel-plugin-wp-i18n\n"

#: blocks/components/editable/format-toolbar.js:105
#: blocks/components/editable/format-toolbar.js:103
msgid "Link"
msgstr ""

#: blocks/components/editable/format-toolbar.js:18
#: blocks/components/editable/format-toolbar.js:16
msgid "Bold"
msgstr ""

#: blocks/components/editable/format-toolbar.js:23
#: blocks/components/editable/format-toolbar.js:21
msgid "Italic"
msgstr ""

#: blocks/components/editable/format-toolbar.js:28
#: blocks/components/editable/format-toolbar.js:26
msgid "Strikethrough"
msgstr ""

Expand Down