-
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 6 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 @@ | ||
export const Blockquote = 'blockquote'; |
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' } } > | ||
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 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> | ||
); | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,6 +3,10 @@ | |
*/ | ||
import { __ } from '@wordpress/i18n'; | ||
import { AlignmentToolbar, BlockControls, RichText } from '@wordpress/block-editor'; | ||
/** | ||
* Internal dependencies | ||
*/ | ||
import { Blockquote } from './blockquote'; | ||
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. 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 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 shared my thoughts already:
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. You will have to run into the same issue when porting Pullquote block. It is also something that plugin developers will need as well. 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. What I meant is that, while this gets translated into a 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 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; | ||
|
@@ -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 | ||
|
@@ -54,7 +58,7 @@ export default function QuoteEdit( { attributes, setAttributes, isSelected, merg | |
className="wp-block-quote__citation" | ||
/> | ||
) } | ||
</blockquote> | ||
</Blockquote> | ||
</> | ||
); | ||
} |
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; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
export { default as withViewportMatch } from './with-viewport-match'; | ||
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 don't think we need this one anymore right? |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,44 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import { mapValues } from 'lodash'; | ||
|
||
/** | ||
* WordPress dependencies | ||
*/ | ||
import { createHigherOrderComponent } from '@wordpress/compose'; | ||
import { withSelect } from '@wordpress/data'; | ||
|
||
/** | ||
* Higher-order component creator, creating a new component which renders with | ||
* the given prop names, where the value passed to the underlying component is | ||
* the result of the query assigned as the object's value. | ||
* | ||
* @see isViewportMatch | ||
* | ||
* @param {Object} queries Object of prop name to viewport query. | ||
* | ||
* @example | ||
* | ||
* ```jsx | ||
* function MyComponent( { isMobile } ) { | ||
* return ( | ||
* <div>Currently: { isMobile ? 'Mobile' : 'Not Mobile' }</div> | ||
* ); | ||
* } | ||
* | ||
* MyComponent = withViewportMatch( { isMobile: '< small' } )( MyComponent ); | ||
* ``` | ||
* | ||
* @return {Function} Higher-order component. | ||
*/ | ||
const withViewportMatch = ( queries ) => createHigherOrderComponent( | ||
withSelect( ( ) => { | ||
return mapValues( queries, ( query ) => { | ||
return query === 'mobile'; | ||
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. As mentioned on Slack, this isn't behaving like the web version. For now, let's make the AlignmentToolbar a fake component on native, and leave that and the viewport implementation for wordpress-mobile/gutenberg-mobile#198 |
||
} ); | ||
} ), | ||
'withViewportMatch' | ||
); | ||
|
||
export default withViewportMatch; |
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?