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

Blocks: Save embed with empty markup when URL not selected #2616

Merged
merged 1 commit into from
Sep 1, 2017

Conversation

aduth
Copy link
Member

@aduth aduth commented Aug 30, 2017

Fixes #2612

This pull request seeks to resolve an issue where a figure with text "undefined" is saved into post content for an embed block, if a URL is not yet assigned to the block.

Open questions:

In cases such as these where a block has a placeholder state, is it sensible to save the block at all, or would it make more sense to omit the block from the saved content? There may be technical challenges in implementing this behavior.

Testing instructions:

  1. Navigate to Gutenberg > New Post
  2. Insert an embed-type block (Vimeo, etc)
  3. Preview the post
  4. Note that the previewed post appears empty
  5. Return to the editor
  6. Refresh the page
  7. Note that the embed block still exists in the post, in its placeholder state

@aduth aduth added the [Feature] Blocks Overall functionality of blocks label Aug 30, 2017
@aduth aduth requested a review from notnownikki August 30, 2017 19:47
@codecov
Copy link

codecov bot commented Aug 30, 2017

Codecov Report

Merging #2616 into master will increase coverage by <.01%.
The diff coverage is 50%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #2616      +/-   ##
==========================================
+ Coverage   31.49%   31.49%   +<.01%     
==========================================
  Files         177      177              
  Lines        5398     5410      +12     
  Branches      943      948       +5     
==========================================
+ Hits         1700     1704       +4     
- Misses       3128     3132       +4     
- Partials      570      574       +4
Impacted Files Coverage Δ
blocks/library/embed/index.js 45.55% <50%> (+0.1%) ⬆️
blocks/library/video/index.js 24.13% <0%> (+3.08%) ⬆️

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 c3b740d...14abbe7. Read the comment docs.

@youknowriad
Copy link
Contributor

would it make more sense to omit the block from the saved content? There may be technical challenges in implementing this behavior

I think the answer to this is that we should save it (like now), There were some suggested use-cases where an author creates the placeholders and another one fill them.

@aduth aduth merged commit 9881b24 into master Sep 1, 2017
@aduth aduth deleted the fix/2612-empty-embed branch September 1, 2017 02:19
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.

Vimeo undefined
2 participants