From 11a2b7270890d1e2719d956aa5a9b2459898d119 Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Wed, 18 Oct 2023 18:12:33 +0530 Subject: [PATCH 1/6] =?UTF-8?q?=F0=9F=90=9B=20Fix=20`amp-story-captions`?= =?UTF-8?q?=20layout=20and=20fix=20`max-height`=20of=20`amp-story-grid-lay?= =?UTF-8?q?er`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `amp-story-captions` adds caption in shadow-dom. Thus, parent `div` container of the `amp-story-captions` won't reflect the actual size of the `amp-story-captions`. This will fix this issue by setting `max-height` of the `amp-story-grid-layer.align-bottom` exactly to the shadow-dom component. Additionally, fixed-height layout of the `amp-story-captions` is changed to container in order to occupy only required space. --- packages/output/src/page.js | 6 +++--- packages/output/src/utils/styles.js | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/output/src/page.js b/packages/output/src/page.js index 8dfc1180ec09..ecd6887323cc 100644 --- a/packages/output/src/page.js +++ b/packages/output/src/page.js @@ -197,13 +197,13 @@ function OutputPage({ aspect-ratio={ASPECT_RATIO} class="grid-layer align-bottom" > -
+ {/* `backgroundColor` is added only for testing purpose. */} +
{videoCaptions.map((captionId) => ( ))}
diff --git a/packages/output/src/utils/styles.js b/packages/output/src/utils/styles.js index 45288ea1e12e..cc18b1d755e1 100644 --- a/packages/output/src/utils/styles.js +++ b/packages/output/src/utils/styles.js @@ -135,6 +135,7 @@ function CustomStyles() { amp-story-grid-layer.align-bottom { align-content: end; padding: 0; + max-height: calc(100vh - 148px); } .captions-area { From ac7445ab226de871aeae9eabd5ed13028cbbbdb1 Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:36:03 +0530 Subject: [PATCH 2/6] =?UTF-8?q?=F0=9F=93=96=20Add=20necessary=20comments,?= =?UTF-8?q?=20=E2=8F=AA=20Revert=20test=20code?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- packages/output/src/page.js | 5 ++--- packages/output/src/utils/styles.js | 8 +++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/packages/output/src/page.js b/packages/output/src/page.js index ecd6887323cc..a1f78a266dbf 100644 --- a/packages/output/src/page.js +++ b/packages/output/src/page.js @@ -197,13 +197,12 @@ function OutputPage({ aspect-ratio={ASPECT_RATIO} class="grid-layer align-bottom" > - {/* `backgroundColor` is added only for testing purpose. */} -
+
{videoCaptions.map((captionId) => ( ))}
diff --git a/packages/output/src/utils/styles.js b/packages/output/src/utils/styles.js index cc18b1d755e1..85d0da2ccdfe 100644 --- a/packages/output/src/utils/styles.js +++ b/packages/output/src/utils/styles.js @@ -135,7 +135,13 @@ function CustomStyles() { amp-story-grid-layer.align-bottom { align-content: end; padding: 0; - max-height: calc(100vh - 148px); + /* + AMP CTA Layer will exactly occupy 74px regardless of any device. + To space out captions 74px from the BOTTOM (AMP CTA Layer), + 74px from the TOP should also be spaced out and thus: 2 * 74px + will be the desired max-height. + */ + max-height: calc(100vh - (2 * 74px)); } .captions-area { From 19e4446f8143f119c3094c7c159bd13b036b277a Mon Sep 17 00:00:00 2001 From: Anurag Vasanwala <75766877+AnuragVasanwala@users.noreply.github.com> Date: Fri, 27 Oct 2023 18:39:02 +0530 Subject: [PATCH 3/6] =?UTF-8?q?=E2=9C=A8=20Add=20`style-preset=3D"default"?= =?UTF-8?q?`=20for=20`amp-story-captions`?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- includes/KSES.php | 3 ++- packages/output/src/page.js | 3 ++- packages/output/src/utils/styles.js | 16 ---------------- 3 files changed, 4 insertions(+), 18 deletions(-) diff --git a/includes/KSES.php b/includes/KSES.php index 9e8e6026d92d..7d902f7f4045 100644 --- a/includes/KSES.php +++ b/includes/KSES.php @@ -571,7 +571,8 @@ public function filter_kses_allowed_html( $allowed_tags ) { 'title' => true, ], 'amp-story-captions' => [ - 'height' => true, + 'height' => true, + 'style-preset' => true, ], 'amp-story-shopping-attachment' => [ 'cta-text' => true, diff --git a/packages/output/src/page.js b/packages/output/src/page.js index a1f78a266dbf..1b8d25f22969 100644 --- a/packages/output/src/page.js +++ b/packages/output/src/page.js @@ -200,9 +200,10 @@ function OutputPage({
{videoCaptions.map((captionId) => ( ))}
diff --git a/packages/output/src/utils/styles.js b/packages/output/src/utils/styles.js index 85d0da2ccdfe..40534c52c986 100644 --- a/packages/output/src/utils/styles.js +++ b/packages/output/src/utils/styles.js @@ -152,22 +152,6 @@ function CustomStyles() { margin-bottom: 16px; text-align: center; } - - amp-story-captions span { - display: inline-block; - margin: 0; - padding: 6px 12px; - vertical-align: middle; - border-radius: 15px; - background: rgba(11, 11, 11, 0.6); - color: rgba(255, 255, 255, 1); - font-family: -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, Oxygen-Sans, Ubuntu, Cantarell, "Helvetica Neue", sans-serif;; - font-size: calc(4 * var(--story-page-vw)); - line-height: 1.4; - word-break: break-word; - word-wrap: break-word; - overflow-wrap: break-word; - } `, }} /> From 2a87ecb9385852ad817726db6aa7504fb12a87c6 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 27 Oct 2023 16:33:07 +0200 Subject: [PATCH 4/6] Slightly improve appearance in the editor --- .../src/components/canvas/mediaCaptions/trackRenderer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/story-editor/src/components/canvas/mediaCaptions/trackRenderer.js b/packages/story-editor/src/components/canvas/mediaCaptions/trackRenderer.js index 6ccd4bc68681..6cf88b565d45 100644 --- a/packages/story-editor/src/components/canvas/mediaCaptions/trackRenderer.js +++ b/packages/story-editor/src/components/canvas/mediaCaptions/trackRenderer.js @@ -55,7 +55,7 @@ function TrackRenderer({ elementId, trackIndex }) { // To mimic the responsive `amp-story-captions span` styling on the frontend. const fontSize = pageWidth * 0.04; - const height = pageHeight * 0.2; + const height = pageHeight * 0.3; const [track, setTrack] = useState(null); const [videoTime, setVideoTime] = useState(0); From e809906250e41c8ba3d6cc16d3f08eba3837d884 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 27 Oct 2023 16:34:43 +0200 Subject: [PATCH 5/6] Update snapshots --- packages/output/src/test/__snapshots__/page.js.snap | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/output/src/test/__snapshots__/page.js.snap b/packages/output/src/test/__snapshots__/page.js.snap index 1b52b717f059..4a8d909528db 100644 --- a/packages/output/src/test/__snapshots__/page.js.snap +++ b/packages/output/src/test/__snapshots__/page.js.snap @@ -2,9 +2,9 @@ exports[`Page output background audio should add background audio as amp-video 1`] = ` `; From 292eb55095e4261f73eab4ed3653cd1725932ba6 Mon Sep 17 00:00:00 2001 From: Pascal Birchler Date: Fri, 27 Oct 2023 16:57:52 +0200 Subject: [PATCH 6/6] Update snapshot --- packages/output/src/test/page.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/output/src/test/page.js b/packages/output/src/test/page.js index c1ba6592d72b..6fde6c269dd3 100644 --- a/packages/output/src/test/page.js +++ b/packages/output/src/test/page.js @@ -1621,9 +1621,9 @@ describe('Page output', () => { await expect(captions).toBeInTheDocument(); expect(captions).toMatchInlineSnapshot(` `); });