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

Hide alignment controls and video toggles #2295

Merged
merged 3 commits into from
May 14, 2019

Conversation

swissspidy
Copy link
Collaborator

@swissspidy swissspidy commented May 13, 2019

Fixes #2115 (again).
See #2283.

Video block:

Screenshot 2019-05-13 at 15 37 16

Image block:

Screenshot 2019-05-13 at 15 39 47

Pullquote block:

Screenshot 2019-05-13 at 15 39 28

Embed block:

Screenshot 2019-05-13 at 15 40 28

@googlebot googlebot added the cla: yes Signed the Google CLA label May 13, 2019
@westonruter
Copy link
Member

Can this be done for the non-story editor as well? We should to be able to hide the “Plays inline” toggle for the video block in non-story posts with this:

diff --git a/assets/css/amp-editor-blocks.css b/assets/css/amp-editor-blocks.css
index 976c2f6d..3abb6b44 100644
--- a/assets/css/amp-editor-blocks.css
+++ b/assets/css/amp-editor-blocks.css
@@ -18,3 +18,10 @@
 	flex: 0 0 auto;
 	margin-right: 1rem;
 }
+
+/**
+ * See https://github.com/ampproject/amp-wp/issues/2283
+ */
+.edit-post-layout[data-block-name="core/video"] .edit-post-settings-sidebar__panel-block .block-editor-block-inspector__card + div > .components-panel__body:first-child .components-toggle-control:nth-child(6) {
+	display: none;
+}

Currently it is displayed:

image

Nevertheless, if someone did by chance have the attribute supplied in an old post, then there would be no way to turn it off. So maybe we should just leave this toggle visible in the non-story editor. What do you think?

@swissspidy
Copy link
Collaborator Author

So maybe we should just leave this toggle visible in the non-story editor. What do you think?

Perhaps for non-story posts we could extend the logic to only hide the controls when they haven't been toggled. And perhaps also only when in native mode.

We can merge this for now to fix the issue for stories, and leave #2283 open to further investigate non-story posts if we think it makes sense.

@westonruter westonruter merged commit 605f1c8 into amp-stories-redux May 14, 2019
@westonruter westonruter deleted the amp-story/image-alignment-v2 branch May 14, 2019 04:17
@westonruter westonruter added this to the v1.2 milestone May 21, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes Signed the Google CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants