OpenTable Block

[scruffian]

This PR adds an OpenTable block, providing block-editor native equivalent functionality of the existing OpenTable embed.

It supports embed code pasted from the OpenTable widget creator:
https://www.opentable.com/widget/reservation/preview?rid=1&lang=en-US

Changes proposed in this Pull Request:

• This PR adds a new block, jetpack/opentable.

Is this a new feature or does it add/remove features to an existing part of Jetpack?

New feature.

Internal reference: pb5gDS-er-p2

The goal would be to release it as a beta block first, in Jetpack 8.1.
Internal reference: p1576850286103100-slack-explorers

Testing instructions:

This is currently marked as a beta block, you will need to add define( 'JETPACK_BETA_BLOCKS', true ); to your wp-config.php file.

• Open the Block Editor.
• Try pasting in an embed code from https://www.opentable.com/widget/reservation/preview?rid=1&lang=en-US
• Try editing the block settings in the sidebar

Proposed changelog entry for your changes:

• Block Editor: Added a OpenTable block.

[jetpackbot]

	Warnings
⚠️	pre-commit hook was skipped for one or more commits

This is an automated check which relies on PULL_REQUEST_TEMPLATE. We encourage you to follow that template as it helps Jetpack maintainers do their job. If you think 'Testing instructions' or 'Proposed changelog entry' are not needed for your PR - please explain why you think so. Thanks for cooperation 🤖

Generated by 🚫 dangerJS against 294447f

[davemart-in]

Nice work getting this stubbed out!

Here's what I'm currently seeing:

In comparing the this current state of this PR to the design mockups we've still got quite a few things to clean up:

Would it be helpful for me to go through and itemize the changes that still need to be made, or do you want to take another pass at cleaning things up first?

[zinigor]

Because this is still in progress as of now, I'm going to move this to the next milestone.

[Copons]

There are a few inconsistencies on the front end rendering, likely caused by some different CSS opinions between OpenTable and TwentyTwenty, that I think we should address.

These should be a good start, but I'm not exactly sure how to enqueue in the "Jetpack way" front-end styles (view or style files) for dynamic blocks.

.ot-dtp-picker {
  box-sizing: content-box;

  .ot-title {
    margin: 4px auto 12px auto;
  }

  .ot-dtp-picker-selector-link {
    text-decoration: none;
  }

  input[type='submit'] {
    text-transform: none;
    padding: 0;
  }
}

[Copons]

I'm wondering about the UX of the restaurant picker with the FormTokenField which only displays the ID.
A few months ago, I've had a similar issue on an old (now removed) FSE feature: it was a post selector for a given CPT with an autocomplete popover list, which displayed the post title, and stored its ID.

If I'm not mistaken, this is the last version of that component before its removal: https://github.com/Automattic/wp-calypso/blob/3932cbd44a2a26897d233d094c9f44ad231c8ba8/apps/full-site-editing/full-site-editing-plugin/full-site-editing/components/post-autocomplete/index.js

What do y'all think if we adapted it (and modernized it with React hooks as well!) to use the OpenTable API?

[scruffian]

What do y'all think if we adapted it (and modernized it with React hooks as well!) to use the OpenTable API?

Sounds like a good idea!

[pablinos]

I was wondering about the UX of displaying just the ID, and I ran up against some problems.

Using the FormTokenField, I thought we could have it display the restaurant name and the ID, but the problem is that it gets a bit long sometimes.

Also, when I looked into doing that, I couldn't find a way of retrieving specific restaurant details by ID from the API endpoint we're using. There are a few problems with that, but one of the biggest is if someone enters their ID and it's a subset of another, like 432 and 4321. You nearly always get back restaurants with 432 in the name, street address, or with IDs starting and ending in 432, but never the restaurant with ID 432.

The only official OpenTable API I could find didn't seem to offer this information either, but if it did, we'd have to proxy the requests and I'm not sure how that would work with access tokens etc. It looks like you have to become a partner to use their Consumer API. We might want to go that route depending on what's involved.

As a side thought, would it be worth adding the features of your post autocomplete component to the FormTokenField, or maybe the whole thing as a new component to @wordpress/components? It seems like this would be generally useful.

[huguesvincent]

For the record, I filled this form back in December and approached a few folks at Open Table without any success so far. I'll keep trying to get on that whitelist.

[Copons]

Ah, I forgot about my PostAutocomplete component, it's not a good fit for this as it expects 1 selected results, while OpenTable allows for multiple.

Also, when I looked into doing that, I couldn't find a way of retrieving specific restaurant details by ID from the API endpoint we're using.

@pablinos Right, damn!
We could partly solve this by storing the restaurant name as a block attribute (not super elegant, but at least I don't expect folks to save dozens of restaurants in their OT blocks?), but that wouldn't help when pasting the embed code, which doesn't contain the restaurant names at all.

if someone enters their ID and it's a subset of another, like 432 and 4321. You nearly always get back restaurants with 432 in the name, street address, or with IDs starting and ending in 432, but never the restaurant with ID 432.

This is rather annoying.
We could be more transparent, and stop saying that the restaurant ID is an accepted value:
Something like this:

- Enter your restaurant name, OpenTable Restaurant ID or embed code
+ Enter your restaurant name, or paste an OpenTable Reservation Widget embed code

Changes were addressed

[jeherve]

I think this should be good to go for now. I created issues for feedback that was discussed above and other things that I found while testing, but that are not blockers for this PR. I think we should be able to address those issues in follow-up PRs.

I'll merge this now.

#14344
#14346
#14335
#14345
#14343
#14347