🐛 [amp story] Pagination forward button visibility state #37098
Conversation
|
Hey @gmajoulet, @newmuis! These files were changed: |
| /** @private @const {!Element} */ | ||
| this.buttonElement_ = devAssert(this.element.firstElementChild); | ||
| /** @const {!Element} */ | ||
| this.buttonElement = devAssert(this.element.firstElementChild); |
There was a problem hiding this comment.
We should probably keep this property private since it's not being used elsewhere and the difference between paginationButton.element and paginationButton.buttonElement is not immediately clear
| toggleEnabled_(paginationButton, isVisible) { | ||
| const {buttonElement, element} = paginationButton; | ||
| element?.classList.toggle('i-amphtml-story-button-hidden', !isVisible); | ||
| buttonElement?.toggleAttribute('disabled', !isVisible); |
There was a problem hiding this comment.
Does the buttonElement need to be disabled if element is already hidden?
There was a problem hiding this comment.
It does since the element is still visible, it just has a lowered opacity (I updated the naming). Button elements will receive focus unless disabled is explicitly set on them.
There was a problem hiding this comment.
Added a couple of nits.
Also note to myself (for possible cleanup in the future): we could reduce the complexity of this code if we don't have the nested elements and just have one HTML button element for each, I bet we can simplify a lot of the styles (eg: use [disabled] instead of i-amphtml-story-button-hidden, get rid of class ..-button-move, etc).
When the attachment is opened, the icon in the forward button disappear when the
HIDDENstate is applied.This is due to the SVG being applied through the state class.
This PR:
HIDDENfrom state, considering it a modifier rather than state.demo one panel
demo supports landscape
Background:
Prior to the desktop one-panel UI the pagination buttons disappeared when the attachment opened.
In the new UI the buttons fade to .1 opacity.
This was not noticed when developing the desktop one-panel UI. This may have been due to the attachment demo being in landscape mode. A one-panel story has been added /examples to help assist with developing for attachments in both UI.