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

Pullquote: Adding block alignments and using proper placeholder #1268

Merged
merged 6 commits into from
Jun 21, 2017

Conversation

youknowriad
Copy link
Contributor

closes #1221
closes #1222

This PR enhances the pullquote block by adding block level alignment controls and is using a proper placeholder instead of default values.

@youknowriad youknowriad added the [Feature] Blocks Overall functionality of blocks label Jun 19, 2017
@youknowriad youknowriad self-assigned this Jun 19, 2017
@jasmussen
Copy link
Contributor

Nice! Thanks for working on this.

When left floated, there's an issue with the layout. I think we need to port the following from being image only, to being for all things left aligned:

max-width: 370px;
width: 100%;

If you remove the placeholder text in the main body of the quote, you get an empty pullquote entirely:

screen shot 2017-06-20 at 10 22 55

This is an interesting state. If you empty out the main field we should at least have "Write quote..." in the main. But the edgecase here is: do we also want it to show when the block is unselected?

@youknowriad
Copy link
Contributor Author

@jasmussen I've fixed the placeholder behaviour and I could use some help with the float left styling issue. I'm not sure I understand correctly and I'm not really aware of how these positioning work right now.

@jasmussen
Copy link
Contributor

Nice!

I pushed a little alignment fix. More work needs doing, but I'll ticket/do that separately. I think we should get this in.

@youknowriad youknowriad requested review from nylen and aduth June 20, 2017 18:58
! content ||
! content.length ||
// On non-inline editables, TinyMCE appends an empty <p> tag
( ! this.props.inline && content.length === 1 && ! content[ 0 ].props.children ),
Copy link
Member

Choose a reason for hiding this comment

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

Could we hook into TinyMCE's internal empty checking?

https://github.com/tinymce/tinymce/blob/master/src/core/src/main/js/dom/Empty.js

I was wondering why we wouldn't have had to account for br[data-mce-bogus] which is added similar to empty <p> tag here. Appears we do some clean-up in getContent already. Should we be stripping the empty paragraphs there?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thihs works great 👍

onChange={
( nextValue ) => setAttributes( {
value: nextValue,
} )
}
placeholder={ ( 'Write Quote…' ) }
Copy link
Member

Choose a reason for hiding this comment

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

You forgot __ 😄

Copy link
Contributor Author

Choose a reason for hiding this comment

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

hahaha, hard to notice because it works

@youknowriad youknowriad merged commit 3c9f47a into master Jun 21, 2017
@youknowriad youknowriad deleted the update/pullquote-alignments branch June 21, 2017 14:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Pullquote: Needs to use proper placeholder text component Pullquote: Needs block level alignments
3 participants