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

Add block descriptions to blocks that miss them. #2427

Merged
merged 12 commits into from Sep 28, 2017

Conversation

Projects
None yet
4 participants
@jasmussen
Contributor

jasmussen commented Aug 15, 2017

This PR, though it looks big, just adds descriptions to every block that misses it, bringing our stash of blocks up to snuff with the design doc.

That also means it adds inspector controls to blocks that don't have any yet. Incidentally, I failed to do this for the "Custom HTML" block, and the "Classic Text" block. I got errors that my skills fell short of fixing. Would appreciate help in addressing those.

Finally, I got a few JS with the Code block and the Gallery block. I'm pretty sure they are errors I introduced, but I can't quite wrap my head around what I did wrong, would really appreciate insights here, thank you. (Feel free to push directly to this branch if need be)

screen shot 2017-08-15 at 19 06 11

@jasmussen jasmussen added the Blocks label Aug 15, 2017

@jasmussen jasmussen self-assigned this Aug 15, 2017

@jasmussen jasmussen requested review from mtias and aduth Aug 15, 2017

@codecov

This comment has been minimized.

Show comment
Hide comment
@codecov

codecov bot Aug 15, 2017

Codecov Report

Merging #2427 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2427      +/-   ##
==========================================
- Coverage   33.77%   33.77%   -0.01%     
==========================================
  Files         190      190              
  Lines        5690     5691       +1     
  Branches      995      996       +1     
==========================================
  Hits         1922     1922              
  Misses       3189     3189              
- Partials      579      580       +1
Impacted Files Coverage Δ
blocks/library/list/index.js 8.13% <ø> (ø) ⬆️
blocks/library/pullquote/index.js 33.33% <ø> (ø) ⬆️
blocks/library/text-columns/index.js 33.33% <ø> (ø) ⬆️
blocks/library/shortcode/index.js 50% <ø> (ø) ⬆️
blocks/library/quote/index.js 16.66% <ø> (ø) ⬆️
blocks/library/button/index.js 26.66% <ø> (ø) ⬆️
blocks/library/separator/index.js 60% <ø> (ø) ⬆️
blocks/library/table/index.js 41.66% <ø> (ø) ⬆️
blocks/library/video/index.js 21.05% <0%> (ø) ⬆️
blocks/library/preformatted/index.js 50% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9210e01...8154b7b. Read the comment docs.

codecov bot commented Aug 15, 2017

Codecov Report

Merging #2427 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2427      +/-   ##
==========================================
- Coverage   33.77%   33.77%   -0.01%     
==========================================
  Files         190      190              
  Lines        5690     5691       +1     
  Branches      995      996       +1     
==========================================
  Hits         1922     1922              
  Misses       3189     3189              
- Partials      579      580       +1
Impacted Files Coverage Δ
blocks/library/list/index.js 8.13% <ø> (ø) ⬆️
blocks/library/pullquote/index.js 33.33% <ø> (ø) ⬆️
blocks/library/text-columns/index.js 33.33% <ø> (ø) ⬆️
blocks/library/shortcode/index.js 50% <ø> (ø) ⬆️
blocks/library/quote/index.js 16.66% <ø> (ø) ⬆️
blocks/library/button/index.js 26.66% <ø> (ø) ⬆️
blocks/library/separator/index.js 60% <ø> (ø) ⬆️
blocks/library/table/index.js 41.66% <ø> (ø) ⬆️
blocks/library/video/index.js 21.05% <0%> (ø) ⬆️
blocks/library/preformatted/index.js 50% <0%> (ø) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9210e01...8154b7b. Read the comment docs.

@aduth

This comment has been minimized.

Show comment
Hide comment
@aduth

aduth Aug 15, 2017

Member

First warning I believe exists already. Might be the table or list block?

Second warning is pending fix at #2000, though hasn't seen much traction lately.

Member

aduth commented Aug 15, 2017

First warning I believe exists already. Might be the table or list block?

Second warning is pending fix at #2000, though hasn't seen much traction lately.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

I would appreciate a final review of this one. With this merged in (thanks so much Riad for the rebase), I think we can close #2023 as fixed, and open separate tickets for the "Custom HTML" block, the "Classic Text" block, and the "Embed" block.

Contributor

jasmussen commented Sep 28, 2017

I would appreciate a final review of this one. With this merged in (thanks so much Riad for the rebase), I think we can close #2023 as fixed, and open separate tickets for the "Custom HTML" block, the "Classic Text" block, and the "Embed" block.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Sep 28, 2017

Contributor

Hey looks like Ella just merged a description for the separator block #2252 we'll have to rebase :)

Contributor

youknowriad commented Sep 28, 2017

Hey looks like Ella just merged a description for the separator block #2252 we'll have to rebase :)

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

Early bird gets the worm. I'll try a rebase :) I think it'll be easier this time.

Contributor

jasmussen commented Sep 28, 2017

Early bird gets the worm. I'll try a rebase :) I think it'll be easier this time.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

Does this rebase to include your HR description look okay @iseulde?

Contributor

jasmussen commented Sep 28, 2017

Does this rebase to include your HR description look okay @iseulde?

@iseulde

This comment has been minimized.

Show comment
Hide comment
@iseulde

iseulde Sep 28, 2017

Member

Oh, you're also updating it 😶 sorry. Sure, but I'm not sure if everyone gets "thematic break". I've just pulled the description form the official w3 docs.

Member

iseulde commented Sep 28, 2017

Oh, you're also updating it 😶 sorry. Sure, but I'm not sure if everyone gets "thematic break". I've just pulled the description form the official w3 docs.

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

Hah, I did the same. I think both descs are fine, I'm fine with either one. Just wanted to make sure the key stuff was alright.

Contributor

jasmussen commented Sep 28, 2017

Hah, I did the same. I think both descs are fine, I'm fine with either one. Just wanted to make sure the key stuff was alright.

@youknowriad

This comment has been minimized.

Show comment
Hide comment
@youknowriad

youknowriad Sep 28, 2017

Contributor

@jasmussen no key issues, everything seems fine

Contributor

youknowriad commented Sep 28, 2017

@jasmussen no key issues, everything seems fine

@jasmussen jasmussen merged commit 8a7bf7f into master Sep 28, 2017

3 checks passed

codecov/project 33.77% (-0.01%) compared to 9210e01
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jasmussen jasmussen deleted the add/block-description branch Sep 28, 2017

@jasmussen

This comment has been minimized.

Show comment
Hide comment
@jasmussen

jasmussen Sep 28, 2017

Contributor

🎉

Contributor

jasmussen commented Sep 28, 2017

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment