[RNMobile] Search Block: Basic UI #29045
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 11 commits
February 12, 2021 20:15
Only adds a shell of a block.
This alert will be removed once styling is added in a later PR.
This will keep the code cleaner and easier to follow.
This toolbar action is now visible in the UI so the temporary alert message is no longer needed.
Was causing issues with layout in iOS. Styling will be addressed fully in a different PR.
|
Size Change: 0 B Total Size: 1.38 MB ℹ️ View Unchanged
|
7 tasks
AmandaRiu
changed the base branch from
master
to
rnmobile/3073-search-block-toolbar
Base automatically changed from
rnmobile/3073-search-block-toolbar
to
master
February 22, 2021 14:11
2 tasks
AmandaRiu
force-pushed
the
rnmobile/3073-search-block-basic-ui
branch
from
4d3fe33 to
58ba580
Compare
AmandaRiu
added
Mobile App - i.e. Android or iOS
| <RichText | ||
| className="wp-block-search__label" | ||
| style={ { | ||
| ...styles.searchLabel, |
cameronvoell
approved these changes
Feb 25, 2021
Member
cameronvoell
left a comment
cameronvoell
left a comment
There was a problem hiding this comment.
Tested in WPAndroid, looks really good! I'll keep an eye out on the known issues and the button position changes in future PR. This looks and works great though 🎉
enejb
reviewed
Feb 25, 2021
| const getBlockClassNames = () => { | ||
| return classnames( | ||
| className, | ||
| 'button-inside' === buttonPosition |
Contributor
There was a problem hiding this comment.
I think some of this function could be simplified by creating a map
Something like
const buttonPostionToClassMap = {
'button-inside' : 'wp-block-search__button-inside',
'button-outside': 'wp-block-search__button-outside',
'no-button': 'wp-block-search__no-button',
'button-only': 'wp-block-search__button-only',
}
let noButtonClass = null;
if( 'no-button' !== buttonPosition ) {
noButtonClass = buttonUseIcon ? 'wp-block-search__text-button' : 'wp-block-search__text-button';
}
return classnames(
className,
buttonPosition && buttonPostionToClassMap [ buttonPosition ],
noButtonClass
);
Let me know what you think :)
Contributor
Author
There was a problem hiding this comment.
Thanks for this @enejb ! This PR is already merged, but I have many more in the pipeline so I'll use this code in the future!
2 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR builds on two previous PRs #28983 and #28985 (
those two have to be merged before this one can be taken out of draftdone!). This PR adds the block UI components and simple styling. Full styling won't be added until a later PR when designs have been finalized. Working features include:Not included:
Screen.Recording.2021-02-16.at.2.47.53.PM.mov
Gutenberg mobile PR
wordpress-mobile/gutenberg-mobile#3186
Known issues
RichTextand supports formatting but for some odd reason color changes only seem to work in the WordPress app, and not in the demo. Example:How has this been tested?
Test the following scenarios and verify the component updates appropriately:
Types of changes
Part of a new feature to port the Search block to mobile
Checklist: