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
adding gradients to all sides #11951
Conversation
[desktop] .next-container { | ||
right: 0; | ||
background: rgba(33,33,33,0)!important; | ||
background: -moz-linear-gradient(left, rgba(33,33,33,0) 0%, rgba(33,33,33,0.98) 100%)!important; |
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.
Remove vendor prefixes.
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.
Done
@@ -314,16 +314,23 @@ export class AmpStory extends AMP.BaseElement { | |||
buildButtons_() { | |||
const doc = this.element.ownerDocument; | |||
const nextButton = doc.createElement('button'); | |||
const nextButtonContainer = doc.createElement('div'); |
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.
nit: this DOM tree is getting too complicated for a series of createElement
's. Consider using utilities found in ./simple-template
(see system-layer.js
for an example).
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.
Done 👍
[desktop] .next-container { | ||
right: 0; | ||
background: rgba(33,33,33,0)!important; | ||
background: -moz-linear-gradient(left, rgba(33,33,33,0) 0%, rgba(33,33,33,0.98) 100%)!important; |
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.
Remove vendor prefixes.
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.
Done
I discussed with Hong and and corrected the gradient values.. all fine now ✌️ |
this.element.insertBefore( | ||
renderSimpleTemplate(this.win.document, PAGE_SWITCH_BUTTONS), | ||
this.element.firstChild | ||
); |
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.
nit: );
on previous line
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.
Done
…orderGradients
@@ -54,6 +54,13 @@ | |||
<p>This is the second page of this story.</p> | |||
</amp-story-grid-layer> | |||
</amp-story-page> | |||
|
|||
<amp-story-page id="page-3"> | |||
<amp-story-grid-layer template="vertical"> |
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.
nit: indentation is off from here down
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.
Done
* adding gradients to all sides * fixing choppy gradient * removing vendor prefixes * render buttons from simpleTemplate * correct gradient values * fixing indents
* adding gradients to all sides * fixing choppy gradient * removing vendor prefixes * render buttons from simpleTemplate * correct gradient values * fixing indents
* adding gradients to all sides * fixing choppy gradient * removing vendor prefixes * render buttons from simpleTemplate * correct gradient values * fixing indents
Adds gradient to all sides of page as mentioned in #11848 last point
cc: @alanorozco @newmuis @choumx