also looking at this
Promote main to production: signed-out chrome and the subscriber tables
#17
54 files
+4252
-1353
Review threads live on the whole diff, not on one commit, so none are shown here - a thread's line means something in the branch's final form, and painting it into an intermediate step would put it on code it is not about.
| @@ -130,6 +130,81 @@ for (const file of filesIn(resources, /\.(?:stx|html|ts)$/)) { | ||
| 130 | 130 | } |
| 131 | 131 | } |
| 132 | 132 | |
| 133 | /* | |
| 134 | * Rule 11.5: no style="" for anything a utility can express. | |
| 135 | * | |
| 136 | * A hand-written declaration on an element is invisible to Crosswind, which | |
| 137 | * extracts from class= attributes only, so it cannot be deduped, purged or | |
| 138 | * themed -- and it silently outranks every utility and shortcut on that | |
| 139 | * element, because an inline style beats any layer. | |
| 140 | * | |
| 141 | * Three shapes are legitimate and exempt. Each was measured, not assumed: | |
| 142 | * | |
| 143 | * style="width: %" a value computed per render and unbounded: a | |
| 144 | * bar height, a progress width. The rule names | |
| 145 | * this case. It stays a mustache rather than | |
| 146 | * becoming the :style binding the rule suggests, | |
| 147 | * because these are SERVER values and :style is | |
| 148 | * evaluated in a browser where the server's data | |
| 149 | * does not exist. | |
| 150 | * | |
| 151 | * :style="progress()" the client-bound form, for a value the browser | |
| 152 | * computes. | |
| 153 | * | |
| 154 | * style="display:none" beside owned by the runtime, and NOT convertible. | |
| 155 | * :show= or :hidden= setVisible() in dist/signals.js reads | |
| 156 | * node.style.display -- the INLINE value, not | |
| 157 | * the computed one -- remembers it, and unhides | |
| 158 | * by writing it back, mapping a 'none' it finds | |
| 159 | * there to ''. So class="hidden" plus :show | |
| 160 | * would unhide to display:'' and fall straight | |
| 161 | * back to the class: hidden forever. The inline | |
| 162 | * none is what makes the element correct before | |
| 163 | * hydration and restorable after it. | |
| 164 | * | |
| 165 | * Two directories are excluded, both because no stylesheet of ours reaches | |
| 166 | * them: | |
| 167 | * | |
| 168 | * resources/emails/ an HTML email has no stylesheet that can reach | |
| 169 | * it and mail clients honour inline CSS only. | |
| 170 | * | |
| 171 | * a component directory that the chart components are distributed to | |
| 172 | * ships its own .css applications that have never heard of | |
| 173 | * Crosswind, so they carry charts.css and every | |
| 174 | * class they use has to be in it (enforced by | |
| 175 | * tests/feature/chart-components.test.ts). A | |
| 176 | * utility class there is INERT, which is the | |
| 177 | * silent-layout-bug case that test exists for -- | |
| 178 | * it caught four legend swatches converted to | |
| 179 | * bg-[color-mix(...)] during this very sweep. | |
| 180 | * The colour of a swatch is the one thing that | |
| 181 | * must survive with no stylesheet at all, so | |
| 182 | * inline is the portable spelling there, and | |
| 183 | * moving it would mean regenerating six | |
| 184 | * distributed artifacts to say the same thing. | |
| 185 | * Detected by a sibling .css rather than by | |
| 186 | * path, so the next such component is covered | |
| 187 | * without being named. | |
| 188 | */ | |
| 189 | const shipsOwnStylesheet = (file: string): boolean => | |
| 190 | files.some(other => other.endsWith('.css') && other.slice(0, other.lastIndexOf('/')) === file.slice(0, file.lastIndexOf('/'))) | |
| 191 | ||
| 192 | for (const file of markupFiles) { | |
| 193 | if (file.includes('/emails/') || shipsOwnStylesheet(file)) | |
| 194 | continue | |
| 195 | const source = withoutComments(readFileSync(file, 'utf8')) | |
| 196 | for (const match of source.matchAll(/(?:^|[^:\w-])style="([^"]*)"/g)) { | |
| 197 | const value = match[1] | |
| 198 | if (value.includes('{{')) | |
| 199 | continue | |
| 200 | const tag = source.slice(source.lastIndexOf('<', match.index), source.indexOf('>', match.index) + 1) | |
| 201 | if (/^\s*display:\s*none;?\s*$/.test(value) && /:(?:show|hidden)=/.test(tag)) | |
| 202 | continue | |
| 203 | const shown = value.length > 48 ? `${value.slice(0, 48)}...` : value | |
| 204 | report(file, source, match.index, `inline style="${shown}" belongs in a class (stx-standards 11.5)`) | |
| 205 | } | |
| 206 | } | |
| 207 | ||
| 133 | 208 | if (!/(?:focus-visible|:focus|focus:|focus-within)/.test(combined)) |
| 134 | 209 | issues.add('resources: no visible keyboard focus treatment found') |
| 135 | 210 | /* |