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

Embed Block Bottom Sheet to set a link #3436

Merged
merged 10 commits into from
Jul 1, 2021

Conversation

ceyhun
Copy link
Contributor

@ceyhun ceyhun commented Apr 29, 2021

Fixes #3274

To test: WordPress/gutenberg#31343

PR submission checklist:

  • I have considered adding unit tests where possible.
  • I have considered if this change warrants user-facing release notes more info and have added them to RELEASE-NOTES.txt if necessary.

@cameronvoell
Copy link
Contributor

Hey @ceyhun . 👋🏻 We will cut the 1.52.0 release Friday 30 April. Please let me know if you're aiming to include this PR in 1.52.0. Otherwise perhaps we can add the milestone label for 1.53.0 Thanks! 🙇

@peril-wordpress-mobile
Copy link

peril-wordpress-mobile bot commented Jun 28, 2021

Wanna run full suite of Android and iOS UI tests? Click here and 'Approve' CI job!

@fluiddot fluiddot requested a review from hypest June 28, 2021 11:15
@fluiddot
Copy link
Contributor

fluiddot commented Jul 1, 2021

@hypest The GB-mobile PR of "Embed Block Bottom Sheet to set a link" is ready for review, it only contains the Gutenberg ref and a release notes update, thanks 🙇 !

@@ -1,5 +1,6 @@
Unreleased
------
* [*] Embed Block Bottom Sheet to set a link [https://github.com/wordpress-mobile/gutenberg-mobile/pull/3436]
Copy link
Contributor

Choose a reason for hiding this comment

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

Oh, my understanding is that the embed block is still behind the dev flag, right? If we keep it that the release notes mention will be confusing, right?

Copy link
Contributor

Choose a reason for hiding this comment

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

Good point, yeah it's behind a dev flag so makes sense not to include it in the release notes 👍 .

I verified if we already included entries related to the embed block and I found this one:

* [*] Generic Embed Block placeholder UI [https://github.com/wordpress-mobile/gutenberg-mobile/pull/3346]

So probably we should remove it too, wdyt?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hmm, yeah, let's remove them. Technically speaking, those were already published to the Stores, but better to remove from the release notes files. Thanks for reviewing the past entries!

Copy link
Contributor

Choose a reason for hiding this comment

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

I've just removed all the entries in the release notes related to the embed block in this commit.

@hypest
Copy link
Contributor

hypest commented Jul 1, 2021

@hypest The GB-mobile PR of "Embed Block Bottom Sheet to set a link" is ready for review, it only contains the Gutenberg ref and a release notes update, thanks 🙇 !

Oops, sorry about missing to review this gb-mobile side PR. Left a comment here.

@hypest hypest mentioned this pull request Jul 1, 2021
2 tasks
Copy link
Contributor

@hypest hypest left a comment

Choose a reason for hiding this comment

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

LGTM!

@hypest hypest enabled auto-merge July 1, 2021 11:56
@hypest hypest merged commit ae506f8 into develop Jul 1, 2021
@hypest hypest deleted the add/3274-embed-block-bottom-sheet branch July 1, 2021 13:18
@fluiddot fluiddot added this to the 1.57.0 (17.8) milestone Jul 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Embed block] Implement Bottom Sheet to set a link
4 participants