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.

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

Port blockquote block to RN mobile #15482

Merged
merged 18 commits into from
May 24, 2019
Merged
Show file tree
Hide file tree
Changes from 16 commits
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
6 changes: 6 additions & 0 deletions docs/manifest-devhub.json
Original file line number Diff line number Diff line change
Expand Up @@ -833,6 +833,12 @@
"markdown_source": "../packages/components/src/popover/README.md",
"parent": "components"
},
{
"title": "BlockQuotation",
"slug": "block-quotation",
"markdown_source": "../packages/components/src/primitives/block-quotation/README.md",
"parent": "components"
},
{
"title": "HorizontalRule",
"slug": "horizontal-rule",
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
const AlignmentToolbar = () => {
return null;
};

export default AlignmentToolbar;
1 change: 1 addition & 0 deletions packages/block-editor/src/components/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ export { default as BlockEdit } from './block-edit';
export { default as BlockFormatControls } from './block-format-controls';
export * from './colors';
export * from './font-sizes';
export { default as AlignmentToolbar } from './alignment-toolbar';
export { default as InspectorControls } from './inspector-controls';
export { default as PlainText } from './plain-text';
export {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -883,6 +883,7 @@ export class RichText extends Component {
RichText.defaultProps = {
formattingControls: [ 'bold', 'italic', 'link', 'strikethrough' ],
format: 'string',
tagName: 'div',
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is replicating the behaviour of the web where there the default tagName is a Div.

This allows the blockquote block to work properly because the

are surrounded by the div.

Our other blocks should work correctly because they define a tagName and don't use the default.

Copy link
Contributor

Choose a reason for hiding this comment

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

are surrounded by the div

Hmm, how do the divs play a role in the native mobile case, considering that this change is in the .native.js file? 🤔

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If you look at the file we expect a tagName in several places to be defined in order to be able to produce the HTML. It also allows the wrapping of the multiple

that can exist in a blockquote inside a root tag.

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, so it's for proper support of the save method of the block?

Let's add this expanded explanation to the PR description @SergioEstevao , cool?

};

const RichTextContainer = compose( [
Expand Down Expand Up @@ -922,6 +923,11 @@ const RichTextContainer = compose( [
selectionStart.clientId === clientId &&
selectionStart.attributeKey === identifier
);
} else {
isSelected = isSelected && (
Copy link
Contributor

Choose a reason for hiding this comment

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

This change (the addition of the "else" block with its setting of the isSelected flag) seems to be causing wordpress-mobile/gutenberg-mobile#1031. @SergioEstevao , do you recall what this change was added to do? I'm going to open a PR to remove it but, I'm afraid I might be missing some context and the removal might add some other problem.

Copy link
Contributor

Choose a reason for hiding this comment

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

@SergioEstevao added a comment about this here: #15833 (comment)

selectionStart.clientId === clientId &&
selectionStart.attributeKey === identifier
);
}

return {
Expand Down
1 change: 1 addition & 0 deletions packages/block-library/src/index.native.js
Original file line number Diff line number Diff line change
Expand Up @@ -113,6 +113,7 @@ export const registerCoreBlocks = () => {
nextpage,
separator,
list,
quote,
].forEach( ( { metadata, name, settings } ) => {
registerBlockType( name, {
...metadata,
Expand Down
5 changes: 3 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,7 @@
*/
import { __ } from '@wordpress/i18n';
import { AlignmentToolbar, BlockControls, RichText } from '@wordpress/block-editor';
import { BlockQuotation } from '@wordpress/components';
Copy link
Contributor

@youknowriad youknowriad May 24, 2019

Choose a reason for hiding this comment

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

I wonder if we should gather all these Platform specific components in a separate package or otherwise under the same parent variable.

Whether it's import { Blockquote, Div } from '@wordpress/platform'

or

import { PlatformComponents } from '@wordpress/components';
const { Blockquote, Div } = PlatformComponents;

Copy link
Member

@gziolo gziolo May 24, 2019

Choose a reason for hiding this comment

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

Yep, this is a good point. I think we should extract all those components into their own package and maybe re-export from @wordpress/components for backward compatibility only. It's definitely out of the scope of this PR @SergioEstevao. Anyway, it's something we should decide on quite soon as you will need to use more HTML tags like:

  • figure, img
  • ul, ol, li
  • div


export default function QuoteEdit( { attributes, setAttributes, isSelected, mergeBlocks, onReplace, className } ) {
const { align, value, citation } = attributes;
Expand All @@ -16,7 +17,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg
} }
/>
</BlockControls>
<blockquote className={ className } style={ { textAlign: align } }>
<BlockQuotation className={ className } style={ { textAlign: align } }>
<RichText
identifier="value"
multiline
Expand Down Expand Up @@ -54,7 +55,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg
className="wp-block-quote__citation"
/>
) }
</blockquote>
</BlockQuotation>
</>
);
}
6 changes: 6 additions & 0 deletions packages/components/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
## 7.5.0
SergioEstevao marked this conversation as resolved.
Show resolved Hide resolved

### New Feature

- Add new `BlockQuotation` block to the primitives folder to support blockquote in a multiplatform way. [#15482](https://github.com/WordPress/gutenberg/pull/15482).

## 7.4.0 (2019-05-21)

### New Feature
Expand Down
15 changes: 15 additions & 0 deletions packages/components/src/primitives/block-quotation/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
# HorizontalRule

A drop-in replacement for the HTML [blockquote](https://developer.mozilla.org/en-US/docs/Web/HTML/Element/blockquote) element that works both on the web and in the mobile apps. It indicates that the enclosed text is an extended quotation.

## Usage

```jsx
import { BlockQuotation } from '@wordpress/components';

const MyBlockQuotation = () => (
<BlockQuotation>
...Quote content
</BlockQuotation>
);
```
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const BlockQuotation = 'blockquote';
16 changes: 16 additions & 0 deletions packages/components/src/primitives/block-quotation/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
/**
* External dependencies
*/
import { View } from 'react-native';
/**
* Internal dependencies
*/
import styles from './style.scss';

export const BlockQuotation = ( props ) => {
return (
<View style={ styles.wpBlockQuote } >
{ props.children }
</View>
);
};
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
.wpBlockQuote {
border-left-width: 4px;
border-left-color: $black;
border-left-style: solid;
padding-left: 8px;
margin-left: 8px;
}
1 change: 1 addition & 0 deletions packages/components/src/primitives/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,3 @@
export * from './svg';
export * from './horizontal-rule';
export * from './block-quotation';
1 change: 1 addition & 0 deletions packages/viewport/src/index.native.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@