Skip to content

Business Hours block: provide defaults #11596

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.

Sign up for GitHub

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

Jump to bottom
Merged
roccotripaldi merged 3 commits into from
Mar 19, 2019
Merged

Business Hours block: provide defaults #11596

roccotripaldi merged 3 commits into from
Mar 19, 2019

Conversation

@roccotripaldi
Copy link
Contributor

@roccotripaldi roccotripaldi commented Mar 18, 2019 •
edited
Loading

Fixes Automattic/wp-calypso#31533

Changes proposed in this Pull Request:

The editor portion of the Business Hours block provides defaults to make the editor experience work before any attributes are set. If folks try to preview the block before configuring them, the front end is blank, because the front end doesn't have any default attributes.

This PR remedies that by using the same defaults that the editor uses.

Testing instructions:

  • Spin up a Jurassic.ninja site using this link
  • Set-up Jetpack
  • Start a new post, and add a business hours block
  • Before making any changes to the block, click "Preview"
  • Ensure the preview looks like you'd expect

Proposed changelog entry for your changes:

  • Fixes a bug in the Business Hours block when blocks with the default attributes failed to appear on the front end

@roccotripaldi
Business Hours Block
96b131f 
provide defaults if no data is provided.
@roccotripaldi
Business hours block
3f5a7af 
Add a doc-block
@roccotripaldi roccotripaldi added [Type] Bug When a feature is broken and / or not performing as intended [Block] Business Hours labels Mar 18, 2019
@roccotripaldi roccotripaldi added this to the 7.2 milestone Mar 18, 2019
@roccotripaldi roccotripaldi requested review from a team March 18, 2019 20:53
@matticbot
Copy link
Contributor

matticbot commented Mar 18, 2019

Caution: This PR has changes that must be merged to WordPress.com
Hello roccotripaldi! These changes need to be synced to WordPress.com - If you 're an a11n, please commandeer, review, and approve D25713-code before merging this PR. Thank you!

@roccotripaldi roccotripaldi added [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. [Status] Needs Review This PR is ready for review. and removed [Status] Needs Copy Review Copy has been added. Marketing will be notified for a copy review. labels Mar 18, 2019
@jetpackbot
Copy link
Collaborator

jetpackbot commented Mar 18, 2019 •
edited
Loading

Thank you for the great PR description!

When this PR is ready for review, please apply the [Status] Needs Review label. If you are an a11n, please have someone from your team review the code if possible. The Jetpack team will also review this PR and merge it to be included in the next Jetpack release.

Scheduled Jetpack release: April 2, 2019.
Scheduled code freeze: March 26, 2019

Generated by 🚫 dangerJS against 901a4f9

@enejb
Copy link
Member

enejb commented Mar 19, 2019

I tested this and it worked as expected. Nicely done.
It fixed the error for me. I was able to replicate the error as well and test that this PR fixes as described.

I wonder it it would make sense to split the code up into a class with static methods?
So this might also help with any future development.

Instead of introducing another function.

I think the change can be done in a different PR as well.

jeherve
jeherve requested changes Mar 19, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This seems to work well. I only have one minor comment

@jeherve jeherve added [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack and removed [Status] Needs Review This PR is ready for review. labels Mar 19, 2019
@matticbot
Copy link
Contributor

matticbot commented Mar 19, 2019

roccotripaldi, Your synced wpcom patch D25713-code has been updated.

@roccotripaldi roccotripaldi added [Status] Needs Review This PR is ready for review. and removed [Status] Needs Author Reply We need more details from you. This label will be auto-added until the PR meets all requirements. labels Mar 19, 2019
@roccotripaldi roccotripaldi mentioned this pull request Mar 19, 2019
Closed
@roccotripaldi
Copy link
Contributor Author

roccotripaldi commented Mar 19, 2019 •
edited by kraftbj
Loading

D25713-code was deployed this morning in r189110-wpcom

@jeherve jeherve added [Status] Ready to Merge Go ahead, you can push that green button! and removed [Status] Needs Review This PR is ready for review. labels Mar 19, 2019
jeherve
jeherve approved these changes Mar 19, 2019
View reviewed changes
Copy link
Member

@jeherve jeherve left a comment

Choose a reason for hiding this comment

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

This should be all set now. 👍

@roccotripaldi roccotripaldi merged commit 00ad2ad into master Mar 19, 2019
@ghost ghost removed the [Status] Ready to Merge Go ahead, you can push that green button! label Mar 19, 2019
@kraftbj kraftbj deleted the fix/business-hours-block-defaults branch March 20, 2019 16:00
kraftbj added a commit that referenced this pull request Mar 26, 2019
@kraftbj
CL #11596
6569907
kraftbj added a commit that referenced this pull request Mar 27, 2019
@kraftbj
Add initial 7.2 changelog to master (#11690)
2227bfd 
* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Initial Changelog for 7.2

* Testing list: add mention of IE11 testing

* Add CL for #11224

* Add CL for #11426

* Add CL for #11442

* Add testing instructions for #11224

* Add CL for #11451

* Reclassify CL item

* Add testing instructions for #11451

* Add CL for #11486

* Add CL for #11418

* Add CL for #11524

* Add CL and testing instructions for #11449

* Add CL for #11460

* Add CL for #11520 and #11582

* Add CL for #11531

* Add CL #11644

* Add testing instructions for #11644

* Add testing instructions for #11644

* Add CL for #11618

* Uniform changelog lines

* CL #11679

* CL #11661

* CL #11654

* CL #11645

* CL #11643

* CL #11636

* CL #11635 and for other PHPCS commits

* CL #11627

* CL #11626

* CL #11598

* CL #11596

* Remove nested items for shortcopy. I don't believe the detailed list is helpful

* CL #11570

* CL #11569

* CL #11560

* CL #11558

* CL #11555

* CL #6704

* CL #11298

* CL #11324

* CL #11443

* CL #11484

* CL #11516

* CL #11529

* Expand Ads block enhancement CL item
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jeherve jeherve jeherve approved these changes

Assignees

No one assigned

Labels

[Block] Business Hours [Focus] Blocks Issues related to the block editor, aka Gutenberg, and its extensions developed in Jetpack Touches WP.com Files [Type] Bug When a feature is broken and / or not performing as intended

Projects

None yet

Milestone

7.2

Development

Successfully merging this pull request may close these issues.

Business Hours Block: Shows up while editing a page/post but it's not published

7 participants

@roccotripaldi @matticbot @jetpackbot @enejb @jeherve @kraftbj