Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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
Implement block quote component in web and RN.
  • Loading branch information
SergioEstevao committed May 22, 2019
commit 65072300a9566a5bfc2902d334c8850f50be60e0
1 change: 1 addition & 0 deletions packages/block-library/src/quote/blockquote.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const Blockquote = 'blockquote';
19 changes: 19 additions & 0 deletions packages/block-library/src/quote/blockquote.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
/**
* External dependencies
*/
import { View } from 'react-native';
/**
* Internal dependencies
*/
import styles from './style';

export const Blockquote = ( props ) => {
return (
<View style={ { flex: 1, flexDirection: 'row', alignItems: 'stretch' } } >
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this can be simpler, Views in react native seem to support border styles:

<View style={ styles.wpBlockQuote } >
	{ props.children }
</View>
.wpBlockQuote {
	border-left-width: 4px;
	border-left-color: $black;
	border-left-style: solid;
	padding-left: 8px;
	margin-left: 8px;
}

I'll let Thomas review the exact colors, but the web version seems to be using black. If you're seeing a blue line, that's coming from Twenty Nineteen

<View style={ styles.wpBlockQuoteMargin } />
<View style={ styles.wpBlockQuoteComponents } >
{ props.children }
</View>
</View>
);
};
8 changes: 6 additions & 2 deletions packages/block-library/src/quote/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,10 @@
*/
import { __ } from '@wordpress/i18n';
import { AlignmentToolbar, BlockControls, RichText } from '@wordpress/block-editor';
/**
* Internal dependencies
*/
import { Blockquote } from './blockquote';
Copy link
Contributor

Choose a reason for hiding this comment

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

Following @gziolo's comment on the other PR, I'm not 100% sure about this naming.

I don't think this should be a standalone component, but having Blockquote inside of the quote block is a bit confusing to me. I think I'd suggest something along the lines of "Container" or "Wrapper" but I'd like to hear from @gziolo as well.

Copy link
Member

Choose a reason for hiding this comment

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

I shared my thoughts already:

Ideally, this new component would be renamed to BlockQuotation and placed inside @wordpress/components package (packages/components/primitives/block-quotation.js?) together with the corresponding documentation. See the prior art from @koke for HorizontalRule implemented in #14361.

Related MDN page which I used to reason about the name of the component: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/blockquote

Copy link
Member

Choose a reason for hiding this comment

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

You will have to run into the same issue when porting Pullquote block. It is also something that plugin developers will need as well.

Copy link
Contributor

Choose a reason for hiding this comment

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

What I meant is that, while this gets translated into a blockquote HTML element in the web version, I don't see it being a good standalone "quote" component right now, like the HorizontalRule is.

The current web implementation only looks like a quote because it's (IMO, wrongly) stealing the styles for the quote block, which won't work when we want to do pullquote as well, and the native version is defining a non-configurable style as well.

Also, those styles are only applied to the blockquote because of the wp-block-quote class, but there's no specific mention of blockquote in the CSS. From what I see, all blockquote provides on the web is semantic markup, but nothing more over a generic div. Which is why, more abstractly, I wonder if it would make sense to make this a generic View/Container element, where one can pass a tagName prop for semantic reasons on the web implementation.

We still have the unresolved issue that we have to use inline styles for native, and CSS classes for the web, but otherwise the code could be pretty similar for both:

<View tagName="blockquote" className={ className } style={ style }>
    <RichText
        identifier="value"
        // ...
    />
    { ( ! RichText.isEmpty( citation ) || isSelected ) && (
        <RichText
	    identifier="citation"
            // ...
	/>
    ) }
</View>


export default function QuoteEdit( { attributes, setAttributes, isSelected, mergeBlocks, onReplace, className } ) {
const { align, value, citation } = attributes;
Expand All @@ -16,7 +20,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg
} }
/>
</BlockControls>
<blockquote className={ className } style={ { textAlign: align } }>
<Blockquote className={ className } style={ { textAlign: align } }>
<RichText
identifier="value"
multiline
Expand Down Expand Up @@ -54,7 +58,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg
className="wp-block-quote__citation"
/>
) }
</blockquote>
</Blockquote>
</>
);
}
61 changes: 0 additions & 61 deletions packages/block-library/src/quote/edit.native.js

This file was deleted.

17 changes: 17 additions & 0 deletions packages/block-library/src/quote/style.native.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
.wpBlockQuoteContainer {
flex: 1;
flex-direction: row;
align-items: stretch;
}

.wpBlockQuoteMargin {
width: 2;
background-color: $blue-dark-900;
}

.wpBlockQuoteComponents {
flex: 1;
flex-direction: column;
justify-content: flex-start;
align-items: stretch;
}