Compare commits
3
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0804566736 | ||
|
|
0b15497c09 | ||
|
|
d55f6cce23 |
No files matched your search
@@ -0,0 +1,108 @@
|
|||||||
|
import fs from 'fs';
|
||||||
|
import path from 'path';
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The hero search and the results filter bar are the same component in two
|
||||||
|
* costumes. `.filterBar` is the card — background, border, shadow, padding —
|
||||||
|
* and `.heroMode` strips all of it so the search sits directly on the hero
|
||||||
|
* panel.
|
||||||
|
*
|
||||||
|
* Both selectors have specificity (0,1,0), so **source order decides**, and
|
||||||
|
* `.heroMode` only wins because it is declared immediately after. Any later
|
||||||
|
* bare `.filterBar` rule — which in practice means one inside a media query —
|
||||||
|
* silently wins instead, and the hero grows a card's padding back.
|
||||||
|
*
|
||||||
|
* That is exactly what happened: `@media (max-width: 768px) { .filterBar {
|
||||||
|
* padding: 0.875rem } }` re-added 14px in hero mode, indenting the search box,
|
||||||
|
* the hint and the location link 14px past the headline above them and costing
|
||||||
|
* the search field 28px of width on a 390px screen. The two rules directly
|
||||||
|
* below it in the same block were correctly written as
|
||||||
|
* `.filterBar:not(.heroMode)`; this one was missed, and nothing caught it
|
||||||
|
* because the result is a plausible-looking layout rather than a broken one.
|
||||||
|
*/
|
||||||
|
|
||||||
|
const CSS = path.join(__dirname, '..', '..', 'components', 'FilterBar.module.css');
|
||||||
|
|
||||||
|
/** Properties `.heroMode` resets. A later bare `.filterBar` rule setting any
|
||||||
|
* of these puts the card back on the hero. */
|
||||||
|
const RESET_BY_HERO_MODE = [
|
||||||
|
'background', 'border', 'border-radius', 'box-shadow', 'padding',
|
||||||
|
];
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Comments are stripped before anything is parsed.
|
||||||
|
*
|
||||||
|
* A `{` or `}` inside a comment would otherwise desynchronise the brace walk
|
||||||
|
* below and the rule regex alike, and the selector text captured for each rule
|
||||||
|
* would carry the preceding comment along with it.
|
||||||
|
*/
|
||||||
|
function withoutComments(css: string): string {
|
||||||
|
return css.replace(/\/\*[\s\S]*?\*\//g, '');
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* The individual selectors in a rule's prelude.
|
||||||
|
*
|
||||||
|
* Split on commas, because a selector list is a list: `.filterBar, .other { }`
|
||||||
|
* applies to `.filterBar` just as surely as `.filterBar { }` does, and an
|
||||||
|
* earlier version of this guard compared the whole prelude against the literal
|
||||||
|
* string '.filterBar' — so writing the regression as a comma list, or across
|
||||||
|
* two lines, would have walked straight past it.
|
||||||
|
*/
|
||||||
|
function selectorsOf(prelude: string): string[] {
|
||||||
|
return prelude.split(',').map((sel) => sel.trim().replace(/\s+/g, ' '))
|
||||||
|
.filter(Boolean);
|
||||||
|
}
|
||||||
|
|
||||||
|
function mediaQueryBodies(css: string): string[] {
|
||||||
|
const bodies: string[] = [];
|
||||||
|
const re = /@media[^{]*\{/g;
|
||||||
|
let m: RegExpExecArray | null;
|
||||||
|
while ((m = re.exec(css)) !== null) {
|
||||||
|
// Walk braces from the opening one to find this at-rule's whole body.
|
||||||
|
let depth = 1;
|
||||||
|
let i = m.index + m[0].length;
|
||||||
|
const start = i;
|
||||||
|
while (i < css.length && depth > 0) {
|
||||||
|
if (css[i] === '{') depth++;
|
||||||
|
else if (css[i] === '}') depth--;
|
||||||
|
i++;
|
||||||
|
}
|
||||||
|
bodies.push(css.slice(start, i - 1));
|
||||||
|
}
|
||||||
|
return bodies;
|
||||||
|
}
|
||||||
|
|
||||||
|
describe('FilterBar hero-mode scoping', () => {
|
||||||
|
const css = withoutComments(fs.readFileSync(CSS, 'utf8'));
|
||||||
|
|
||||||
|
it('confirms heroMode still resets the card, which is what makes this matter', () => {
|
||||||
|
const hero = css.match(/\.heroMode\s*\{([^}]*)\}/);
|
||||||
|
expect(hero).not.toBeNull();
|
||||||
|
expect(hero![1]).toMatch(/padding:\s*0/);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('never re-applies card styling to the hero from inside a media query', () => {
|
||||||
|
const offenders: string[] = [];
|
||||||
|
|
||||||
|
for (const body of mediaQueryBodies(css)) {
|
||||||
|
for (const rule of body.matchAll(/([^{}]+)\{([^{}]*)\}/g)) {
|
||||||
|
// Only a *bare* .filterBar is dangerous, and it is dangerous wherever
|
||||||
|
// it appears in a selector list. Scoped variants
|
||||||
|
// (`.filterBar:not(.heroMode)`) and descendants are fine.
|
||||||
|
const selectors = selectorsOf(rule[1]);
|
||||||
|
if (!selectors.includes('.filterBar')) continue;
|
||||||
|
|
||||||
|
for (const prop of RESET_BY_HERO_MODE) {
|
||||||
|
if (new RegExp(`(^|[;\\s])${prop}\\s*:`).test(rule[2])) {
|
||||||
|
offenders.push(`${rule[1].trim()} sets ${prop}`);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Fix by scoping the rule as `.filterBar:not(.heroMode)`, the way the
|
||||||
|
// neighbouring rules in the same block already are.
|
||||||
|
expect(offenders).toEqual([]);
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -413,7 +413,17 @@
|
|||||||
/* ── Narrow ───────────────────────────────────────────────────────── */
|
/* ── Narrow ───────────────────────────────────────────────────────── */
|
||||||
|
|
||||||
@media (max-width: 768px) {
|
@media (max-width: 768px) {
|
||||||
.filterBar {
|
/*
|
||||||
|
* Scoped, like the two rules below it.
|
||||||
|
*
|
||||||
|
* The results filter bar is a card — background, border, shadow — and needs
|
||||||
|
* inner padding. The hero's search is not a card: .heroMode zeroes the
|
||||||
|
* padding, border and background so the search sits directly on the panel.
|
||||||
|
* Unscoped, this rule put 14px back, which indented the search box, the hint
|
||||||
|
* and the location link 14px past the headline they sit under, and cost the
|
||||||
|
* search field 28px of width on a 390px screen.
|
||||||
|
*/
|
||||||
|
.filterBar:not(.heroMode) {
|
||||||
padding: 0.875rem;
|
padding: 0.875rem;
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -457,6 +467,14 @@
|
|||||||
align-items: flex-start;
|
align-items: flex-start;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/* Optical alignment: the button's own 6px of padding is what makes its
|
||||||
|
label start further right than the hint above it, even once both boxes
|
||||||
|
share a left edge. Pulling the padding back off lines the text up while
|
||||||
|
keeping the tap target. */
|
||||||
|
.heroMode .nearMeBtn {
|
||||||
|
margin-left: -0.375rem;
|
||||||
|
}
|
||||||
|
|
||||||
.geoError {
|
.geoError {
|
||||||
text-align: left;
|
text-align: left;
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in new issue
Block a user