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

Slot/Fill pattern with Toolbar #199 #11115

Merged
merged 27 commits into from
Nov 7, 2018
Merged

Conversation

marecar3
Copy link
Contributor

Description

This PR exposes Slot/Fill pattern for gutenberg-mobile. It is tested with gutenberg-mobile PR

Screenshots

screen shot 2018-10-26 at 2 19 54 pm

To test:

@marecar3 marecar3 changed the title Rnmobile/native toolbar component DO NOT MERGE. Slot/Fill pattern with Toolbar #199 Oct 29, 2018
@gziolo
Copy link
Member

gziolo commented Oct 30, 2018

What's blocking it from merging? It looks good 👍

@gziolo
Copy link
Member

gziolo commented Oct 30, 2018

Okey, I see your comment: wordpress-mobile/gutenberg-mobile#199 (comment).

@gziolo gziolo added the Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change) label Oct 30, 2018
@marecar3 marecar3 changed the title DO NOT MERGE. Slot/Fill pattern with Toolbar #199 Slot/Fill pattern with Toolbar #199 Nov 1, 2018
@@ -83,7 +82,9 @@ class ParagraphEdit extends Component {
tagName="p"
value={ content }
style={ {
...style,
// We don't want to pass style here from upper leyers, because it's corrupted
Copy link
Contributor

@hypest hypest Nov 2, 2018

Choose a reason for hiding this comment

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

It would be good if we can re-use the work we did for the Inserter's SVGs case where we take the concatenated string passed in the props and convert it to proper style format. Can we make a utility function out of that work and use it here? (Same for the code block above, or elsewhere)

Copy link
Member

Choose a reason for hiding this comment

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

Is this class name ever converted to style properly with the Babel plugin that's used in the project?

Copy link
Contributor Author

@marecar3 marecar3 Nov 2, 2018

Choose a reason for hiding this comment

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

Hey @hypest @gziolo, I saw the solution for converting style string to style object, but when I look at the input (style string) and output (style object) it doesn't make sense for this case.
This is what I get as a style"custom-class-1 custom-class-2 has-background has-drop-cap has-large-font-size has-vivid-red-background-color"' I think that I don't want that to be converted into style, as I didn't pass it. What do you think about it? I am open for suggestions.

Copy link
Contributor

@koke koke left a comment

Choose a reason for hiding this comment

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

Tested this and it looks good to go :shipit: 🎉

@marecar3 marecar3 merged commit 26ccc15 into master Nov 7, 2018
daniloercoli added a commit that referenced this pull request Nov 7, 2018
…rnmobile/fix-merge-content-not-refreshed-UI

* 'master' of https://github.com/WordPress/gutenberg:
  Fix the isBeingScheduled Selector.  (#11572)
  Slot/Fill pattern with Toolbar #199 (#11115)
  Add mechanism to avoid forced child selection on blocks with templates. (#10696)
  Allow a block to disable being converted into a reusable block; Fix: Column block (#11550)
@mzorz mzorz deleted the rnmobile/native-toolbar-component branch November 14, 2018 12:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mobile App - i.e. Android or iOS Native mobile impl of the block editor. (Note: used in scripts, ping mobile folks to change)
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants