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

🖍 [Story interactive] Styling for Image Quizzes and Polls with Missing Images #35420

Merged
merged 3 commits into from
Jul 27, 2021

Conversation

Brandons42
Copy link
Contributor

Puts a background color in place of a missing image for image quizzes and image polls; the background color is a grey-ish color that depends on the theme of the component

Demos (see second-to-last and third-to-last pages):
https://interactive-img.web.app/missing-img/quiz.html
https://interactive-img.web.app/missing-img/poll.html

/cc @mszylkowski
/cc @processprocess

@amp-owners-bot amp-owners-bot bot requested a review from Enriqe July 27, 2021 19:09
@amp-owners-bot
Copy link

Hey @gmajoulet! These files were changed:

extensions/amp-story-interactive/0.1/amp-story-interactive-img.css
extensions/amp-story/1.0/amp-story.css

Hey @mszylkowski! These files were changed:

extensions/amp-story-interactive/0.1/amp-story-interactive-img.css

Hey @newmuis! These files were changed:

extensions/amp-story/1.0/amp-story.css

@mszylkowski mszylkowski requested review from mszylkowski and processprocess and removed request for Enriqe July 27, 2021 19:38
@@ -924,6 +924,7 @@ amp-story .amp-video-eq,
--i-amphtml-interactive-component-shadow: 0px 4px 12px rgba(60, 64, 67, 0.08), 0px 2px 4px rgba(60, 64, 67, 0.12);
--i-amphtml-interactive-prompt-clamp: 4 !important;
--i-amphtml-interactive-results-prompt-margin: 20px !important;
--i-amphtml-interactive-missing-img-background: rgba(32, 33, 36, 0.2) !important;
Copy link
Contributor

Choose a reason for hiding this comment

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

We can call it --i-amphtml-interactive-placeholder-background so we can reuse these colors on the results component with the text bubbles

Copy link
Contributor

@processprocess processprocess left a comment

Choose a reason for hiding this comment

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

Approving with an optional nit

@@ -924,6 +924,7 @@ amp-story .amp-video-eq,
--i-amphtml-interactive-component-shadow: 0px 4px 12px rgba(60, 64, 67, 0.08), 0px 2px 4px rgba(60, 64, 67, 0.12);
--i-amphtml-interactive-prompt-clamp: 4 !important;
--i-amphtml-interactive-results-prompt-margin: 20px !important;
--i-amphtml-interactive-placeholder-background: rgba(32, 33, 36, 0.2) !important;
Copy link
Contributor

@processprocess processprocess Jul 27, 2021

Choose a reason for hiding this comment

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

nit: This might be more descriptive as:
--i-amphtml-interactive-img-placeholder-background

Copy link
Contributor

Choose a reason for hiding this comment

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

As discussed in chat, we'll keep this as-is since it might be used for other contexts.

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.

4 participants