-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Changes from 17 commits
6f02cba
8dc7c06
116cf9b
ae65dfc
3bc05ac
6507230
4d1de79
f5ebb76
536442e
587a01b
03be5ad
f28f013
a1a16f4
08b2327
d8b77ba
1f8a12a
1f2e231
889116f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
const AlignmentToolbar = () => { | ||
return null; | ||
}; | ||
|
||
export default AlignmentToolbar; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -883,6 +883,7 @@ export class RichText extends Component { | |
RichText.defaultProps = { | ||
formattingControls: [ 'bold', 'italic', 'link', 'strikethrough' ], | ||
format: 'string', | ||
tagName: 'div', | ||
}; | ||
|
||
const RichTextContainer = compose( [ | ||
|
@@ -922,6 +923,11 @@ const RichTextContainer = compose( [ | |
selectionStart.clientId === clientId && | ||
selectionStart.attributeKey === identifier | ||
); | ||
} else { | ||
isSelected = isSelected && ( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 { | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,7 @@ | |
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { AlignmentToolbar, BlockControls, RichText } from '@wordpress/block-editor'; | ||
import { BlockQuotation } from '@wordpress/components'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 or import { PlatformComponents } from '@wordpress/components';
const { Blockquote, Div } = PlatformComponents; There was a problem hiding this comment. Choose a reason for hiding this commentThe 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
|
||
|
||
export default function QuoteEdit( { attributes, setAttributes, isSelected, mergeBlocks, onReplace, className } ) { | ||
const { align, value, citation } = attributes; | ||
|
@@ -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 | ||
|
@@ -54,7 +55,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg | |
className="wp-block-quote__citation" | ||
/> | ||
) } | ||
</blockquote> | ||
</BlockQuotation> | ||
</> | ||
); | ||
} |
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'; |
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; | ||
} |
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'; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
|
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, how do the
div
s play a role in the native mobile case, considering that this change is in the.native.js
file? 🤔There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?