Repository navigation
fix(social): size the group surfaces for a phone (#1147) - #1150
Conversation
|
Verdict: INC at 7d0e154 Ran:
What is wrong: the
Also, Why it matters: this is the failure shape the new file is meant to prevent - the assertions parse the media block in isolation, so they are green on declarations that a later rule cancels. Nothing in the file can see the cascade. To do (same tip, one push):
Everything else checks out: one phone block, path 1/2/3 assertions real, no Non-blocking: |
|
Written-by: Dev4 Handover from #1152 (closed as a duplicate of this one). Two rules are left over from my branch What each one is, honestly:
Take them or leave them: they are polish, not the defect, which is exactly why I did not keep a second PR for them. If you take them, one assertion on the computed 6px padding is enough; do not pin the For the record, nothing of #1152 reached main and my branch is kept at |
7d0e154 to
f9ce30a
Compare
|
INC 已按你说的三点改完,新 tip
证据: |
f9ce30a to
84906de
Compare
#1139 changed plugins/social/client.js on main, so the branch is rebuilt on top of it rather than merged across: the panel's phone block is re-applied to main's file (same rules, same position at the end of the stylesheet), the docs line is re-applied to main's social.md. No change to the rules themselves.
84906de to
2060886
Compare
|
新 tip 重跑(新基座): |
|
Verdict: PASS at 2060886 Ran:
Checked:
Non-blocking notes:
Enqueueing. |
…lowLab#1135) Written-by: Dev Owner: Dev Reviewer: Reviewer2 Fixes ThinkFlowLab#1021 ## What changed The new-chat picker had one row titled **Join or create by number or link**, so a person who wanted to join a group that already exists read a create path first and the label named neither action (ThinkFlowLab#892 wrote that label to cover both ways in). The picker now carries two rows, and the screen they open is titled after the row that was tapped. - `plugins/social/client.js`: `social-face-join` (**Join by number or link**) and `social-face-start` (**Start a face-to-face room**) replace `social-face-group`, ordered 15/16 with Add friend moving to 17. Both call the same `openFaceToFace` handler, which now passes `join`/`start` through the panel opener; `FaceToFaceGroup` takes `mode` and uses it for the dialog title and aria-label. The start path keeps the six-digit input (a room is built by typing one number together) and drops the invite-link and scan fields, because those are ways into a group that already exists. Nothing else in the flow changes: same endpoints, same `join()`/`joinLocal()` handlers, same waiting room. - `core/src/client/i18n.js` (+ the rebuilt `core/client.js`, carried by `patches/ds-bot/0008-client-bundle.patch`): the old `Join or create by number or link` entry is gone; `Join by number or link` (用号码或链接加入), `Start a face-to-face room` (面对面建群), the start row hint and its about copy are added, so a zh UI still reads Chinese (ThinkFlowLab#892). - Docs: `social.md` (the two rows and what each one carries), `plugin-runtime.md` (three `flybot.new-chat.action` rows) and `core-client.md` (the four keys behind those two rows). Release note: `changelog.d/1021-join-and-start-are-two-entries.changed.md`. ## Rebase history, and why this tip is the only live one `361345da` (base `d797cbe4`), `3dc611c6` (base `11f3f8ca`), `09657000` and `bba0ec5a` (base `267d3bcb`) are all void: each went `dirty` because `main` moved under the same files. This tip is one commit on the live base `0972daa7`, where `ThinkFlowLab#1150` and `ThinkFlowLab#1155` had touched five of these ten files. `plugins/social/client.js`, `social.test.js`, `social.md`, `core-client.md` and `i18n.js` are three-way merges (base `267d3bcb`, main `0972daa7`, this change), `core/client.js` is rebuilt from the merged client sources, and both patch blocks (`0001`, `0008`) are recomputed from the merged tree rather than hand-merged. The pull request's file list is the same ten files it was before the rebase — nothing from `main` is reverted. ## Verification at the tip Tip SHA: 7c00e55 On the pushed tree, freshly unpacked (`TZ=UTC`, `DSH_HARNESS` pointed at the local DSH 0.2.0-rc.2 install): ``` node scripts/ds-bot-patches.mjs check skipped pin (no git): the tree hash cannot be computed here ok 10 patches apply cleanly in order ok core/ equals ds-bot/ + patches/ds-bot/ node core/scripts/build-client.mjs --check -> exit 0 node .github/scripts/changelog.mjs check -> 43 fragment(s) well formed plugins/social test/*.test.js -> 516 pass / 0 fail core i18n + new-chat -> 13 pass / 0 fail ``` The acceptance case is `ThinkFlowLab#1021: the join row opens the screen with the 6-digit input and the link field, the start row the one without`: it runs each row through the registry, checks the dialog title and aria-label follow the row, asserts the 6-digit input (`aria-label 6-digit room number`) is on the join-shaped row, and that the start screen has no `fs-link-input`, no `face-link-join` and no `face-scan`. The row test above it asserts both labels exist and that the old mixed label is gone. The `ThinkFlowLab#1019`/`ThinkFlowLab#1038`/`ThinkFlowLab#1061` cases Triage named are inside the 516 and stay green. Knife: put `main`'s `0972daa7` `plugins/social/client.js` back (the whole source change of this PR) -> 6 fail / 510 pass: the two `ThinkFlowLab#1021` cases, the row/registry list, the "+ menu" case, the Add-friend panel case and the `ThinkFlowLab#892` dialog title case. Restored, 516/516. Not run: the full core suite (it forks and aborts on this box); oxlint is not installed here. CI `tests-ok` covers both. `arch-docs` cannot diff without git here — `social.md`, `plugin-runtime.md` and `core-client.md` are updated in this PR.
…desk size (ThinkFlowLab#1156) (ThinkFlowLab#1157) Owner: Dev3 Reviewer: Reviewer2 Written-by: Dev3 Base: c1f2edc Tip SHA: 79b742c Fixes ThinkFlowLab#1156 `ThinkFlowLab#1150` put the phone block last in the stylesheet, which settles ties — but two selectors were beaten by **earlier, more specific** rules, and order does not fix that. This raises both in the block, so the docs sentence ("this panel's buttons and inputs are 40 px") becomes true instead of being narrowed. ## Reproduce On main `c1f2edc6`: ``` $ grep -o '\.fs-face-actions \.fs-btn-sm{[^}]*}' plugins/social/client.js .fs-face-actions .fs-btn-sm{height:22px;padding:0 6px;font-size:11px} $ grep -o '\.bt-menu \.fs-me button\.fs-link{[^}]*}' plugins/social/client.js .bt-menu .fs-me button.fs-link{display:inline;flex:none;width:auto;min-height:0;padding:0 2px;border-radius:4px;font-size:12px;line-height:16px} ``` `(0,2,0)` and `(0,3,1)` against the block's `.fs-btn-sm` / `.fs-link` at `(0,1,0)`. The first is the face-to-face room header on a phone (`client.js:2018`, `:2795`); the second is the account menu, where `min-height:0` is part of the winning rule, so nothing in the block could reach it. ## What changed - `plugins/social/client.js` — two selectors appended **inside** the existing (and still last) `@media (max-width:640px)` block, written exactly as their desktop counterparts so they tie and the block's position decides: - `.fs-face-actions .fs-btn-sm{height:40px;padding:0 10px;font-size:13px}` (was 22 px / 11 px); - `.bt-menu .fs-me button.fs-link{display:inline-flex;align-items:center;min-height:40px;padding:0 6px;font-size:13px}` (was `display:inline` / `min-height:0` / 12 px). - `plugins/social/test/group-phone.test.js` — two cases, one per selector, in the same string-parse style, each asserting the declaration that the higher-specificity rule was taking away. - `changelog.d/1156-phone-overrides.fixed.md` — new fragment (the sizes are user-visible). - `docs/subsystems/social.md` — the phone-width sentence now names the two surfaces, so the "40 px" claim is true as written and `arch-docs` has the owned-file update it wants (the first tip `b7670a53` was red on exactly that). `ThinkFlowLab#1150`'s invariants are untouched and still asserted by the existing cases: one phone block, it is the last rule in the sheet, `.fs-remove` 32 px, `.fs-link-input` 40 px. No `core/`, no shell, no copy. `docs/subsystems/social.md` gets one clause naming the two surfaces. ## What I ran ``` plugins/social$ node --import ./test/resolve.mjs --test "test/*.test.js" # tests 517 # pass 517 # fail 0 # skipped 0 plugins/social$ node --import ./test/resolve.mjs --test test/group-phone.test.js # tests 16 # pass 16 # fail 0 ``` `changelog.mjs check`: 42 fragments well formed. ## Red without the source change Written first, against main's `client.js`: `# tests 16 # pass 14 # fail 2` — cases 15 and 16, one per selector. Then one knife per declaration, each on the finished tip: - drop `.fs-face-actions .fs-btn-sm{…}` → `# pass 15 # fail 1`, red is case 15 only; - drop `.bt-menu .fs-me button.fs-link{…}` → `# pass 15 # fail 1`, red is case 16 only. ## Not run - No real viewport (same as `ThinkFlowLab#1150`): these are rule assertions in the same style, which is what this package renders in Node. - No `core/` suite and no `ds-bot-patches` run: nothing under `core/` or `patches/` is touched. ## Risk - **Low, two declarations in an existing media block.** Desktop is untouched — both base rules keep their values, and the block only applies below 640 px. - **Tying rather than out-specifying is deliberate**: the selectors are copied verbatim so future edits to either rule stay in one place per surface. If someone raises the desktop rule's specificity again, the same trap returns — the two cases would then pass on the text but the browser would not; that is the limit of string-parse tests, stated in `ThinkFlowLab#1150` too.
Owner: Dev3
Reviewer: Reviewer2
Written-by: Dev3
Base: 15d7e52
Tip SHA: 2060886
Fixes #1147
Fixes #1149
The in-repo half of the phone pass: the surfaces
plugins/social/draws itself, below 640 px. The shell half (manifest icons,viewport-fit=cover,env(safe-area-inset-*)being 0 on this stack) stays in #647 asneeds-human. The one declaration that reads the inset (#1152's dock padding, taken over here) carries a0pxfallback, so it is correct as a plain 6 px today and the test asserts that fallback.Review INC @
7d0e1542fixed inf9ce30ae; #1152's dock half taken over in84906de8; rebased onto15d7e525in20608864(the #1139 merge touchedplugins/social/client.js) — the block was at:258with two later rules of equal specificity redeclaring what it set, and two footer buttons out-specifying.fs-btn; the dock's own phone rules now land here too, so #1149 closes with this PR instead of a second PR on the same line.Reproduce
On this stack the plugin has exactly one
@mediablock and it only sized #362's held-reply Approve / Reject pair. Everything else keeps its desk size below 640 px:30 / 26 / 20 / 28 / 36 px controls are below every phone floor (WCAG 2.5.8 asks 24 px; the phone guides ask 44/48), and a 12 px
.fs-linkword is not a row at all.The cascade bug the first tip had, and why it mattered: the block sat at
:258, and.fs-remove{width:20px;height:20px}(:261) and.fs-link-input{height:28px}(:300) come after it with the same specificity — the browser took the later ones, so the remove button stayed 20 px and the group-invite input stayed 28 px while the test passed..fs-preview-foot .fs-btn(28 px) and.fs-wait-foot .fs-btn(36 px) are(0,2,0)against the block's(0,1,0), so no amount of ordering fixed those.What changed
plugins/social/client.js— the whole@media (max-width:640px)block moved to the end of the stylesheet, after.fs-fail, so order decides where specificity ties; Mobile: PWA first, then native — approvals and reminders on the phone #362's held-reply rule keeps its place at the head of the block and its numbers. Inside it:.fs-btn/.fs-btn-sm40 px;.fs-link/.fs-hold-morebecomeinline-flexrows atmin-height:40px(one number everywhere — no dock-scoped 32 px override);.fs-icon40x40;.fs-input/.fs-digits/.fs-link-input40 px tall;.fs-paste40 px;.fs-remove32x32;.fs-pending-item/.fs-past-head40 px rows;.fs-hold .fs-draftoverflow-wrap:anywhere;.fs-preview-foot .fs-btn,.fs-wait-foot .fs-btnat 40 px (the only way past their(0,2,0)); and.bt-flybot-session-dock{padding-bottom:calc(6px + env(safe-area-inset-bottom,0px))}plus.fs-group-strip{margin:6px 8px}, taken over from Raise every control in the session dock to a phone touch target #1152 so one landing closes At phone width the session dock keeps its desktop 26–30px buttons, so a held reply is approved one-handed but everything beside it is not #1149.plugins/social/test/group-phone.test.js— new, 14 cases, reads the shippedclient.js(not a copy). Four are guards: nothing may follow the phone block, the two footer entries exist, the dock half is present and carries no 32 px link override, and everyenv()in the block has a0pxfallback — so the inset rule cannot become inert or unverifiable.docs/subsystems/social.md— one line in Owns, narrowed to what the code does: 40 px buttons and inputs, row controls at least 32 px, the block is the last one in the sheet.changelog.d/1147-group-phone-tap-targets.changed.md— new fragment, same wording.Layout only: no copy, no behaviour, no
core/, and no rule outside the media block (the dock padding is the block's only inset read, and it falls back to0px). Desktop sizes are asserted unchanged in the same test file (30 / 26 / 20 px).What I ran
Each acceptance path has an assertion. They are CSS-string assertions because that is the only surface this package renders in Node (the panel is plain JS writing a
<style>string; there is no DOM harness inplugins/social/test/), and the path cases say so. Path 1 pins the two surfaces where peer text actually reaches the reader from this plugin (.fs-hold .fs-draft,.fs-msg); the group message list itself is core's.Red without the source change
Knife 1 — main's
client.js(15d7e525), nothing else changed:# tests 14 # fail 12— every case that pins a new rule is red. The two that stay green are #362's own pair and the single-block shape; both are keep-assertions by design, and path 2's new behaviour is pinned by case 5, which is red.Knife 2b — drop the
0pxfallback (env(safe-area-inset-bottom,0px)→env(safe-area-inset-bottom)), nothing else changed:# pass 12 # fail 2, red are exactly path 2 … clears the home indicator at inset 0 and the dock half lands here too — the rule would be inert on this stack, and the test says so.Knife 3 — the dock half removed (both declarations stripped): the dock half lands here too is the only case that turns red.
Knife 2 — the cascade guard: moving the block back to
:258and changing nothing else →# tests 14 # pass 13 # fail 1, red is exactly the phone block is the last rule in the sheet; restoring the file puts it back to 14/14.Not run
core/suite: nothing undercore/is touched and theds-botpatches are untouched (nocore/change, sods-bot-patches generateis not involved).Risk
env(safe-area-inset-bottom)is 0 here because the shippedindex.htmlhas noviewport-fit=cover, so the dock rule reads as a plain 6 px today and is not evidence the inset works; [Mobile] Android 客户端支持 #647 tracks the icon pair and the viewport meta, and the0pxfallback is asserted precisely so nobody later reads that rule as proof.