Button: Add "Start on a new line" option (#1473) #1512
Conversation
|
I will pull and rebase to avoid marge commit. |
Codecov Report
@@ Coverage Diff @@
## master #1512 +/- ##
=========================================
- Coverage 33.77% 33.67% -0.1%
=========================================
Files 191 190 -1
Lines 5691 5686 -5
Branches 996 993 -3
=========================================
- Hits 1922 1915 -7
- Misses 3189 3191 +2
Partials 580 580
Continue to review full report at Codecov.
|
|
@gziolo I rebased with master. Below is the problem and the test case: I want to align my button to the right, then next line is a paragraph, like this:
But if I align my button right, it floats and the second paragraph goes beside the button:
With the "Start on a new line" option, the button is always on a line, even when floats left, right. We can discuss about "Start on a new line" phrase. I can't find a better phrase. |
|
related #1473 |
@karmatosed or @jasmussen any suggestions on the name this option? |
|
What about this phrase: "Stand on a line" |
I like that. |
|
Just a note that this is probably something we can handle as an "extension" to blocks, the same way we're thinking of refactoring the custom className and anchor properties. See here for more details #2474 (comment) |
|
Code looks good, it works as advertised. It only needs an option name to be updated as discussed above and it should be good to merge. Thank you very much for working on that one. |
|
@gziolo Thank you. Do I need to update the commit to change option name? |
|
That would be the best. You can also add another commit and we can squash before merge. |
|
I updated the commit with option text is "Stand on a line" |
blocks/library/button/index.js
Outdated
|
|
||
| return [ | ||
| focus && ( | ||
| <BlockControls key="controls"> | ||
| <BlockAlignmentToolbar value={ align } onChange={ updateAlignment } /> | ||
| </BlockControls> | ||
| ), | ||
| focus && ( | ||
| <InspectorControls key="inspector"> | ||
| {clearControl} |
There was a problem hiding this comment.
@truongwp one more little thing before we merge. Can you add spaces around clearControl to satisfy WordPress coding guidelines? I missed that yesterday, thanks!
|
@gziolo I added spaces around |
|
Travis CI seems to be unhappy with this change which is surprising and completely unexpected. I'm trying to find out what has happened on Slack's #core-editor channel. |
|
Sometimes I meet this error. I think it isn't coding error. |
|
I moved "Stand on a line" options to below of block description. |
|
This is how it looks like at the moment:
@jasmussen I'm not familiar with the patterns here, can you confirm it fits well? Another question is if we should remove |
I think we had the 370px value back when we didn't have image resizing, and perhaps the button just inherited it. It seems like if the button width can expand the block border itself, then that would be superior to having a fixed width. What do you think, @mtias? By the way we should reconsider that link thing on the button. I know why we did it initially, but two toolbars is too much for that block. We should have a link button in the toolbar that just applies at the block level to the block. |
|
Okey, it looks like this width issue is unrelated to this change. So let's fix it in another PR if we agree that it makes sense. Merging this one as it is. @truongwp thanks for your contribution 👍 |
There is no way to clear the float when using an align left or right button, the next text button always behinds the button. You can read more here: #1473