-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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
♻️ [amp-story-page-attachment] revise cta text in docs #36426
♻️ [amp-story-page-attachment] revise cta text in docs #36426
Conversation
Hey @gmajoulet, @newmuis! These files were changed:
|
@@ -86,7 +80,7 @@ Contrast protection is automatically applied to ensure readability and a11y comp | |||
<amp-story-page-outlink | |||
layout="nodisplay" | |||
theme="dark"> | |||
<a href="https://www.google.com">Call To Action</a> | |||
<a href="https://www.google.com"></a> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we keep the "Call to action" text here? I believe it can be used to configure the cta-text
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We decided to remove it since the default text is reflected in the image.
The following two examples include text corresponding to the example images.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We still need to document how people can tweak the call to action, should we update the screenshot instead of removing the configuration?
This was not meant to be added to the docs for this component and can be deleted.
A comment is added about why the codepath is necessary in
renderOutlinkUI
The demo text is updated to reflect the text in the images.