ampproject / amp-toolbox-php

AMP Optimizer PHP library
Apache License 2.0
73 stars 25 forks source link

Add SSR transformer which adds `noscript > style[amp-noscript]` with base styles #349

Open westonruter opened 3 years ago

westonruter commented 3 years ago

As of https://github.com/ampproject/amphtml/issues/20609, AMP now supports a noscript > style[amp-noscript] element to include styles specifically targeting pages on which JS is disabled. This is particularly important to ensure that certain AMP components have all their information accessible to such users. For example, in https://github.com/ampproject/amphtml/issues/20609#issuecomment-901426550 there is the example of amp-accordion where all accordion sections should be forcibly expanded in no-JS context, which can be achieved with this CSS:

amp-accordion > section:not([expanded]) > :last-child {
    display: block !important;
}

(Disregard the use of !important not being allowed in AMP, for which I've opened https://github.com/ampproject/amphtml/issues/36051.)

In the same way as there is an AmpRuntimeCss transformer to inline style[amp-custom] there should also be a transformer to populate noscript > style[amp-noscript] with styles that ensure AMP components are able to be rendered accessibly to users with JS disabled. Part of this effort should include contributing base noscript styles to each component which we can then be downloaded from the AMP CDN.

schlessera commented 3 years ago

What is the next step here? Does the responsibility to define these accessible styles lie with the AMP team? Do we want to provide a test implementation right away that can later be ported to the AMP infrastructure?

westonruter commented 3 years ago

I think it can be done either way, yes. We could come up with an initial draft of styles that can be used to provide the initial no-JS compatibility and then add them to the noscript > style[amp-noscript], and then surface those styles back up to the AMP team to be incorporated into each component's as a new *.noscript.css stylesheet.

In any case, this is blocked by https://github.com/ampproject/amphtml/issues/36051, since we'd need to be able to override !important styles.