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

Improve unsupported blocks #643

Merged
merged 22 commits into from
Apr 3, 2019
Merged

Improve unsupported blocks #643

merged 22 commits into from
Apr 3, 2019

Conversation

koke
Copy link
Member

@koke koke commented Feb 22, 2019

Show localized title and icon of unsupported core blocks. It also drops our custom gmobile/unsupported block and moves the functionality to a native variant of core/missing.

Gutenberg PR: WordPress/gutenberg#14577

Simulator Screen Shot - iPhone XS - 2019-03-21 at 11 06 30

To test:

  • Create a post with some blocks that are unsupported in the app.
  • Open it in gutenberg-mobile and verify that the blocks show the right icon and localized block title. For non-core blocks, a generic icon is shown.

Part of #479

koke added 2 commits February 22, 2019 00:46
Make sure the locale data is setup before importing anything, since block names
get translated on import
@koke
Copy link
Member Author

koke commented Feb 22, 2019

I haven't figured out how to set the right colors yet, and then I realized that translations weren't working for block titles, so I focused on that first, but cc @iamthomasbishop since you might want to keep track of this one

@iamthomasbishop
Copy link
Contributor

iamthomasbishop commented Feb 22, 2019

Awesome! I think I might need to update the colors here anyway, as the design changed slightly. IIRC, I just made the icon and text label $gray-dark aka #2e4453, but I'll double check.

Although now that I look at it, unsupported blocks might look okay dimmer like that last block 🤔

@iamthomasbishop
Copy link
Contributor

Just for the sake of consistency and documentation, some specs at the end:

Static:
image

Focused:
image

  • Icon and text label are indeed $gray-dark aka #2e4453
  • Background has a border-radius of 4pt
  • Text label uses system sans-serif font (SF Pro, Roboto) at 14pt
  • Icon is 24pt
  • Margin between icon and text label is 4pt

@hypest hypest added this to the v1.0 milestone Feb 22, 2019
@koke koke removed this from the v1.0 milestone Feb 22, 2019
@Tug
Copy link
Contributor

Tug commented Feb 22, 2019

Did a bit of refactoring, but this LGTM 👍

Btw, this introduces some "Circle dependency" warnings when booting the app in dev mode. We indeed have circle dependencies in our code (often in gutenberg), I think those warnings didn’t show up before because we would load those components before our app component is even loaded but there were still there.

@koke koke added this to the v1.3 milestone Mar 22, 2019
@koke koke requested a review from etoledom March 22, 2019 14:39
@koke koke added the Blocks label Mar 22, 2019
@koke koke changed the title [WIP] Improve unsupported blocks Improve unsupported blocks Mar 22, 2019
Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Hey @koke - This looks and works super good! 🎉

I got curious about the hasUnsupportedBlocks property we are sending back to Native, and for some reason it seems to not be working.

It's easy to see when running the example app from Xcode that it logs: gutenbergDidMount(hasUnsupportedBlocks: false) with the example content.

I noticed that this.blocks is empty on AppContainer's componentDidMount, and that would explain this behavior.

After fixing this detail, I'd say it's good to go.
Great job!

@koke
Copy link
Member Author

koke commented Mar 28, 2019

Since you're handling the hasUnsupportedBlocks issue in #790, should we go ahead and merge this one? (After resolving conflicts 😩)

@etoledom
Copy link
Contributor

Sure! One last smoke test.

Copy link
Contributor

@etoledom etoledom left a comment

Choose a reason for hiding this comment

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

Works good and looks super nice! 🎉
The previously described issue wasn't from this PR, so let's :shipit:

@koke koke merged commit c391c55 into develop Apr 3, 2019
@koke koke deleted the feature/unsupported-core-blocks branch April 3, 2019 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants