From cd2e89f5f5cccdb116db4fcb82d00b6325972782 Mon Sep 17 00:00:00 2001 From: Christophe Besson Date: Tue, 18 Aug 2026 17:46:54 +0200 Subject: fix: the chat tab no longer scrolls, and a group is listed or invite-only MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit **The chat tab was 8px too tall, at every window size.** The panel is sized from JS to `viewport - top - 16`, which puts its bottom 16px above the fold — but it sits inside `.main`, which adds 24px of padding below it. Eight pixels of document past the window, whatever the window. Measured at 700, 900 and 1200: `scrollHeight` 708, 908, 1208. This is the second one of these — the sign-in card was `.page-center` and `.layout` each reserving `100vh - 52px` — so it is now measured in the suite rather than reasoned about. `tests/harness/scroll_probe.py` renders the real markup against the real stylesheet and **runs the real `fit()` lifted out of `app.js`**: a copy of the formula in a test would go on passing after the original changed, which is exactly the bug being guarded. The fix does not encode 24 anywhere. The first pass runs as before, then the leftover is measured and taken off, so anything added below the panel later is absorbed the same way. Now `scrollHeight == innerHeight` at all three heights, nothing below the fold, and the panel still fills the room it has — that last one has its own test, because shrinking the chat to 240px would satisfy every other assertion here and be useless. The Settings tab was measured too and is **not** a bug: it fits at 1200px and overflows only when its content is genuinely taller than the window. **Group creation asked one question twice.** Visibility and admission were separate selectors that could only ever be set together — picking Public reached over and set the policy — and two of the four combinations are meaningless. The API already refused public+invite with a 422, so the form could build a request that could not succeed. Private+open was accepted and should not have been: a group anyone may join that nobody can find is a listing with the listing removed, since joining goes through the node and there is no link to pass around. So: one selector, "who can join", and the request derives the rest. The API now refuses the other impossible pair as well, with a message that says which way to resolve it. Six locale strings the visibility box owned are deleted rather than left unread in ten files, and the two surviving descriptions now say what each choice means for who can *find* the group — with the word "public" gone from the page, nothing else would have said it, and someone would publish a group without meaning to. 865 tests pass. Co-Authored-By: Claude Opus 5 --- CLAUDE.md | 9 ++ packages/meshbay-hub/src/meshbay_hub/api/groups.py | 33 ++-- packages/meshbay-hub/src/meshbay_hub/static/app.js | 80 +++++----- .../src/meshbay_hub/static/locales/de.js | 10 +- .../src/meshbay_hub/static/locales/en.js | 10 +- .../src/meshbay_hub/static/locales/es.js | 10 +- .../src/meshbay_hub/static/locales/fr.js | 10 +- .../src/meshbay_hub/static/locales/it.js | 10 +- .../src/meshbay_hub/static/locales/ja.js | 10 +- .../src/meshbay_hub/static/locales/nl.js | 10 +- .../src/meshbay_hub/static/locales/pl.js | 10 +- .../src/meshbay_hub/static/locales/pt-BR.js | 10 +- .../src/meshbay_hub/static/locales/zh-CN.js | 10 +- packages/meshbay-hub/tests/harness/scroll_probe.py | 169 +++++++++++++++++++++ .../meshbay-hub/tests/test_groups_self_service.py | 45 ++++++ .../meshbay-hub/tests/test_page_does_not_scroll.py | 125 +++++++++++++++ .../meshbay-hub/tests/test_transport_contracts.py | 60 +++++--- 17 files changed, 471 insertions(+), 150 deletions(-) create mode 100644 packages/meshbay-hub/tests/harness/scroll_probe.py create mode 100644 packages/meshbay-hub/tests/test_page_does_not_scroll.py diff --git a/CLAUDE.md b/CLAUDE.md index fcddaf5..a5469a1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -402,6 +402,15 @@ anything that assumes one key per person. for it would have failed these tests for no visible reason. Use `Object.defineProperty`; the suite is now green on 18 and 24 +- **Two subtractions in different files, one scrollbar.** Twice now a page was + permanently a few pixels too tall: `.page-center` and `.layout` each + reserving `100vh - 52px`, then the chat panel sized to `viewport - top - 16` + while `.main` adds 24px of padding underneath it. Neither is visible in the + stylesheet, and both read as correct on their own. `tests/harness/ + scroll_probe.py` measures the document against the window and runs the real + `fit()` lifted out of `app.js` — the sizing code is never reimplemented in a + test, or the test outlives the code it was written for + - **A refusal that never rejects.** Denying Chromium's `fullscreen` permission does not make `requestFullscreen()` throw — the promise never settles. The deny-everything handler was written from a true sentence ("nothing here needs diff --git a/packages/meshbay-hub/src/meshbay_hub/api/groups.py b/packages/meshbay-hub/src/meshbay_hub/api/groups.py index 8283276..6819ff6 100644 --- a/packages/meshbay-hub/src/meshbay_hub/api/groups.py +++ b/packages/meshbay-hub/src/meshbay_hub/api/groups.py @@ -317,17 +317,30 @@ async def create_group( current_user: User = Depends(require_user_scope), db: AsyncSession = Depends(get_db), ): + # Being listed and being open are one question, not two. + # + # A public group that admits nobody is a contradiction: it is in the + # directory, so people find it and then discover they cannot get in. + # Admission by request was considered and dropped — between strangers the + # only channel is the hub, so the one-time code would travel through the + # very party it exists to keep out, and would protect nothing. + # + # The other way round was accepted until now and should not have been: a + # group anyone may join, that nobody can find, is a listing with the listing + # removed. Nothing could reach it but a link, and there is no link — joining + # goes through the node. The create form no longer offers either + # combination; refusing them here is what makes that true of the API too. + if body.visibility == "public" and body.join_policy != "open": + raise HTTPException( + status_code=422, + detail="A public group is open to join. Make it private if you " + "want to choose who comes in.") + if body.visibility != "public" and body.join_policy == "open": + raise HTTPException( + status_code=422, + detail="A private group is invite-only. Make it public if you want " + "anyone to be able to join.") if body.visibility == "public": - # A public group that admits nobody is a contradiction: it is listed in - # the directory, so people find it and then discover they cannot get in. - # Admission by request was considered and dropped — between strangers the - # only channel is the hub, so the one-time code would travel through the - # very party it exists to keep out, and would protect nothing. - if body.join_policy != "open": - raise HTTPException( - status_code=422, - detail="A public group is open to join. Make it private if you " - "want to choose who comes in.") await _check_public_group_quota(db, current_user) desc = (body.description or "")[:512] if body.description else None diff --git a/packages/meshbay-hub/src/meshbay_hub/static/app.js b/packages/meshbay-hub/src/meshbay_hub/static/app.js index eb11469..62bbfa5 100644 --- a/packages/meshbay-hub/src/meshbay_hub/static/app.js +++ b/packages/meshbay-hub/src/meshbay_hub/static/app.js @@ -1005,7 +1005,6 @@ function ExplorePage({ token, myGroupIds }) { function CreateGroupPage({ token, onCreated }) { const [name, setName] = useState(''); const [description, setDescription] = useState(''); - const [visibility, setVisibility] = useState('private'); const [joinPolicy, setJoinPolicy] = useState('invite'); const [error, setError] = useState(''); const [loading, setLoading] = useState(false); @@ -1016,7 +1015,10 @@ function CreateGroupPage({ token, onCreated }) { setLoading(true); setError(''); try { - const body = { name: name.trim(), visibility, join_policy: joinPolicy }; + // Derived, not asked: "open" is what makes a group listed, and there is + // no third combination the server would accept. + const body = { name: name.trim(), join_policy: joinPolicy, + visibility: joinPolicy === 'open' ? 'public' : 'private' }; if (description.trim()) body.description = description.trim().slice(0, 512); const data = await hubFetch('/v1/groups', { method: 'POST', token, body, @@ -1055,59 +1057,39 @@ function CreateGroupPage({ token, onCreated }) { + ${/* One question, not two. Visibility and admission were separate + selectors that could only ever be set together: a public group + admits everyone by definition, and a private one that anyone may + join is a directory listing nobody can find. The server already + refused public+invite with a 422 — the form could build a request + that could not succeed. Now the answer to "who can join" settles + both, and the descriptions say what each one means for who can + *find* the group, which is the part the visibility box was there + to state and no longer needs to. */ html`
-

${t('create_group.visibility')}

+

${t('create_group.join_policy')}

-
- - ${visibility === 'public' - ? html`

- ${t('create_group.public_is_open')} -

` - : html` -

- ${t('create_group.join_policy')} -

-
- - -
- `}
+ `} + MeshBay +
🔔 +
+ +""" + +CHAT_TAB = NAV_AND_SIDEBAR + """ +
+ +
+

a group

+
+ + + +
+
+

hello

+
+
+
+
+""" + +SHORT_PAGE = NAV_AND_SIDEBAR + """ +
+ +
+

a group

+

not much here

+
+
+""" + + +def _measure(fragment: str, tmp_path: Path) -> dict: + path = tmp_path / "fragment.html" + path.write_text(fragment, encoding="utf-8") + proc = subprocess.run( + ["python3", str(HARNESS), str(path), ",".join(str(h) for h in HEIGHTS)], + capture_output=True, text=True, timeout=180) + assert proc.returncode == 0, f"probe failed: {proc.stdout}{proc.stderr}" + out = json.loads(proc.stdout) + assert "error" not in out, f"no measurement: {out}" + return out + + +@pytest.fixture(scope="module") +def chat(tmp_path_factory): + return _measure(CHAT_TAB, tmp_path_factory.mktemp("chat")) + + +@pytest.fixture(scope="module") +def short(tmp_path_factory): + return _measure(SHORT_PAGE, tmp_path_factory.mktemp("short")) + + +@pytest.mark.parametrize("height", HEIGHTS) +def test_the_chat_tab_fits_its_window(chat, height): + r = chat[str(height)] + assert r["overflow"] <= 0, ( + f"the document is {r['overflow']}px taller than the {height}px window — " + f"a scrollbar on the chat tab. Past the fold: {r['past']}") + + +@pytest.mark.parametrize("height", HEIGHTS) +def test_nothing_on_the_chat_tab_hangs_below_the_fold(chat, height): + """The composer is the one that matters: a chat you cannot type in.""" + assert chat[str(height)]["past"] == [] + + +@pytest.mark.parametrize("height", HEIGHTS) +def test_the_chat_panel_uses_the_room_it_has(chat, height): + """The correction must not overshoot. The panel should end just above the + fold, not halfway up the page — a 240px chat in a 1200px window would pass + every assertion above and be useless.""" + panel = chat[str(height)]["panel"] + assert panel, "no chat panel in the measurement" + gap = height - panel["bottom"] + assert 0 <= gap <= 40, ( + f"the panel ends {gap}px above the fold at {height}px") + + +@pytest.mark.parametrize("height", HEIGHTS) +def test_a_short_page_does_not_scroll_either(short, height): + """The control: without this, a chat panel shrunk to nothing would pass.""" + r = short[str(height)] + assert r["overflow"] <= 0, f"{r['overflow']}px of overflow with no content" diff --git a/packages/meshbay-hub/tests/test_transport_contracts.py b/packages/meshbay-hub/tests/test_transport_contracts.py index 8d975d7..5da8104 100644 --- a/packages/meshbay-hub/tests/test_transport_contracts.py +++ b/packages/meshbay-hub/tests/test_transport_contracts.py @@ -129,6 +129,14 @@ def test_presence_has_three_states_and_a_label_for_each(app): "the dot needs a title and an aria-label, not just a colour") +def _string(source: str, key: str) -> str: + """One locale entry's text, whether it is written on one line or spliced + across several with `+`.""" + start = source.index(f"'{key}':") + len(f"'{key}':") + end = source.index("\n '", start) + return source[start:end] + + def test_a_refusal_from_the_node_counts_as_present(app): """The node answering "no" proves it is up; only silence proves nothing.""" assert "err.reason ? 'online' : 'offline'" in app @@ -136,30 +144,48 @@ def test_a_refusal_from_the_node_counts_as_present(app): # ── The create-group form ───────────────────────────────────────────────────── -def test_choosing_public_settles_the_admission_question(app): - """Public implies open, so the policy selector has nothing left to ask. - - Enforced twice on purpose: the API refuses public+invite with a 422, and the - form never offers the combination. A form that can build a request the server - rejects is a form that produces an error message instead of a group. +def test_the_form_asks_one_question_not_two(app): + """ + Visibility and admission were separate selectors that could only ever be set + together, and the form knew it — picking Public reached over and set the + policy. Two of the four combinations were impossible: the API refused + public+invite with a 422, and private+open is a directory listing nobody can + find, joining being through the node rather than a link. + + So there is one selector. "Open" is what makes a group listed, and the + request derives the rest. """ - assert "setVisibility('public'); setJoinPolicy('open');" in app, ( - "picking Public must settle the policy, not leave the previous one") - assert "setVisibility('private'); setJoinPolicy('invite');" in app, ( - "going back to Private must not leave the group open by accident") - form = app[app.index("function CreateGroupPage"):] form = form[:form.index("\n}\n")] - selector = form.index("t('create_group.join_policy')") - guard = form.rindex("visibility === 'public'", 0, selector) - assert guard != -1, "the policy selector must sit behind a visibility guard" - assert "create_group.public_is_open" in form[guard:selector], ( - "a public group should say why there is nothing to choose") + + assert "setVisibility(" not in form, "the visibility selector is back" + assert "t('create_group.join_policy')" in form + assert "joinPolicy === 'open' ? 'public' : 'private'" in form, ( + "the request must derive visibility rather than leave it unset") + + +def test_the_form_says_what_each_choice_means_for_finding_the_group(app): + """Dropping the visibility box removes the words "public" and "private" + from the page. If the descriptions do not say it, nothing does — and + somebody publishes a group without meaning to.""" + en = (STATIC / "locales" / "en.js").read_text(encoding="utf-8") + invite = _string(en, "create_group.invite_desc") + open_ = _string(en, "create_group.open_desc") + assert "not listed" in invite.lower() + assert "listed" in open_.lower() and "anyone" in open_.lower() + + +def test_the_strings_the_visibility_box_used_are_gone(app): + """A key nobody reads is a key that rots, and ten locales carry each one.""" + for locale in (STATIC / "locales").glob("*.js"): + text = locale.read_text(encoding="utf-8") + for key in ("create_group.visibility", "create_group.private", + "create_group.public_is_open", "create_group.public_desc"): + assert f"'{key}'" not in text, f"{locale.name} still carries {key}" def test_the_form_starts_on_a_combination_the_api_accepts(app): form = app[app.index("function CreateGroupPage"):] - assert "useState('private')" in form[:form.index("return html")] assert "useState('invite')" in form[:form.index("return html")] -- cgit v1.2.3