Skip to content
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

Text: add drop cap support #1189

Merged
merged 6 commits into from
Jun 15, 2017
Merged

Text: add drop cap support #1189

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
c51f524
Text: add support for dropCap.
mtias Jun 15, 2017
f09458e
Get around contentEditable issues with first letter.
mtias Jun 15, 2017
49ac22d
Improve display of setting and add label.
mtias Jun 15, 2017
2b98530
Use focus instead of selected to hide dropcap
ellatrix Jun 15, 2017
30dbbb1
Fix missing key.
mtias Jun 15, 2017
d8a83c0
Move dependencies where they belong.
mtias Jun 15, 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
13 changes: 13 additions & 0 deletions blocks/editable/style.scss
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,19 @@
}
}

.drop-cap-true .blocks-editable__tinymce:not( :focus ) {
&:first-letter {
float: left;
font-size: 4.1em;
line-height: 0.7;
font-family: serif;
font-weight: bold;
margin: .07em .23em 0 0;
text-transform: uppercase;
font-style: normal;
}
}

.block-editable__inline-toolbar {
display: flex;
justify-content: center;
Expand Down
22 changes: 19 additions & 3 deletions blocks/library/text/index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
/**
* WordPress dependencies
*/
import { __ } from 'i18n';
import { Children, cloneElement } from 'element';
import Toggle from 'components/form-toggle';

/**
* Internal dependencies
Expand All @@ -10,11 +12,12 @@ import { registerBlockType, createBlock, query, setDefaultBlock } from '../../ap
import AlignmentToolbar from '../../alignment-toolbar';
import BlockControls from '../../block-controls';
import Editable from '../../editable';
import InspectorControls from '../../inspector-controls';

const { children } = query;

registerBlockType( 'core/text', {
title: wp.i18n.__( 'Text' ),
title: __( 'Text' ),

icon: 'text',

Expand All @@ -31,8 +34,8 @@ registerBlockType( 'core/text', {
},

edit( { attributes, setAttributes, insertBlockAfter, focus, setFocus, mergeBlocks } ) {
const { align, content } = attributes;

const { align, content, dropCap } = attributes;
const toggleDropCap = () => setAttributes( { dropCap: ! dropCap } );
return [
focus && (
Copy link
Member

@aduth aduth Jun 15, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Possible alternative to creating single combined focus condition:

focus && ...[
	<BlockControls key="controls">
		<AlignmentToolbar
			value={ align }
			onChange={ ( nextAlign ) => {
				setAttributes( { align: nextAlign } );
			} }
		/>
	</BlockControls>,
	<InspectorControls key="inspector">
		<div className="blocks-text__drop-cap" style={ { display: 'flex', justifyContent: 'space-between' } }>
			<label htmlFor="blocks-text__drop-cap">{ __( 'Drop Cap' ) }</label>
			<Toggle
				checked={ !! dropCap }
				onChange={ toggleDropCap }
				id="blocks-text__drop-cap-toggle"
			/>
		</div>
	</InspectorControls>
],

Copy link
Member Author

Choose a reason for hiding this comment

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

This is something we'll need to pick a model and apply consistently, because people will start using as examples. I personally don't like the arrays much and prefer a single element tree.

Copy link
Member

Choose a reason for hiding this comment

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

In fairness, it's already an array 😄

<BlockControls key="controls">
Expand All @@ -44,6 +47,18 @@ registerBlockType( 'core/text', {
/>
</BlockControls>
),
focus && (
<InspectorControls key="inspector">
<div className="blocks-text__drop-cap" style={ { display: 'flex', justifyContent: 'space-between' } }>
Copy link
Member

Choose a reason for hiding this comment

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

Why do we give this a className but then apply its styles inline? Should we move the styles to styles.scss?

I think elsewhere @youknowriad had mentioned creating better patterns for inspector sections with labels. Doesn't seem like it should be the responsibility of the block to make sure its inspector controls are aligned as desired.

Copy link
Member Author

Choose a reason for hiding this comment

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

Just speed considering all these controls will be superseded by components in following iterations, properly absorbing the styles.

<label htmlFor="blocks-text__drop-cap">{ __( 'Drop Cap' ) }</label>
<Toggle
checked={ !! dropCap }
onChange={ toggleDropCap }
id="blocks-text__drop-cap-toggle"
/>
</div>
</InspectorControls>
),
<Editable
key="editable"
value={ content }
Expand All @@ -62,6 +77,7 @@ registerBlockType( 'core/text', {
} }
onMerge={ mergeBlocks }
style={ { textAlign: align } }
className={ `drop-cap-${ dropCap }` }
Copy link
Member

Choose a reason for hiding this comment

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

Should this be a modifier class on Editable?

className={ classnames( { 'has-drop-cap': dropCap } ) }
.blocks-editable.has-drop-cap .blocks-editable__tinymce:not( :focus ) {

Or better yet, a prop on the component if it's Editable's own styles responsible for creating the effect?

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, plan is to turn it into has-drop-cap. I am not sure if it belong in editable, it feels like purely a text/paragraph addition. (Makes no sense in quotes, captions, etc.)

/>,
];
},
Expand Down