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

More block enhancements #1079

Merged
merged 9 commits into from
Jun 6, 2019
Merged

Conversation

etoledom
Copy link
Contributor

@etoledom etoledom commented Jun 5, 2019

Fixes #859

As mentioned in #859 (comment), I opted for modifying the More block to be read only, in order for it to be ready for the next release.

More info and test steps on the gutenberg side PR: WordPress/gutenberg#16005

On the design side, I noticed on web that More block and Page Break block look the same, so I implemented the same design we use for Page Break on mobile too, but I'll wait for @iamthomasbishop 's input on this.

more-block

Update release notes:

  • If there are user facing changes, I have added an item to RELEASE-NOTES.txt.

@etoledom etoledom added Blocks [Status] Needs Design Review Needs design review or sign-off before shipping labels Jun 5, 2019
@etoledom etoledom added this to the v1.7 milestone Jun 5, 2019
@etoledom etoledom self-assigned this Jun 5, 2019
@etoledom etoledom marked this pull request as ready for review June 5, 2019 14:01
@iamthomasbishop
Copy link
Contributor

Nice work! A couple of little things I wanted to document – although not specific to the Read More block (also applies to the Page Break block):

  • The divider should extend to the ends of the canvas/container
  • The label should be sans-serif
  • Ideally the divider should be a dashed line, but I think that might've been a compromise bc of CSS limitations in the RN app

@etoledom
Copy link
Contributor Author

etoledom commented Jun 5, 2019

Thank you for the review @iamthomasbishop !

The divider should extend to the ends of the canvas/container

I was able to extend the lines a bit more, until the edge of the block content.

We can extend them more using some negative magic margin values, but I'm not sure if that's what you had in mind, and it might not be a very good idea :)

The label should be sans-serif

Ideally the divider should be a dashed line, but I think that might've been a compromise bc of CSS limitations in the RN app

The lines are actually a normal (thin) view with a background color. Not possible to make them dashed with the current component we are using. I'd recommend this to be a future enhancement.

This is an updated screenshot:

more

@koke
Copy link
Member

koke commented Jun 6, 2019

Not possible to make them dashed with the current component we are using. I'd recommend this to be a future enhancement

For reference, some initial effort going (on hold) here: WordPress/gutenberg#14175

Copy link
Member

@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.

If @iamthomasbishop is good with the visuals, the implementation looks good to me

@iamthomasbishop
Copy link
Contributor

Looks great to me! Thanks for the updates @etoledom! :shipit:

@etoledom etoledom removed the [Status] Needs Design Review Needs design review or sign-off before shipping label Jun 6, 2019
@etoledom etoledom merged commit 83ea557 into develop Jun 6, 2019
@etoledom
Copy link
Contributor Author

etoledom commented Jun 6, 2019

Thank you both!

@etoledom etoledom deleted the issue/859-more-block-enhancements branch June 6, 2019 14:27
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.

More block
3 participants