diff --git a/.changeset/8934-chatter-feed-affordance-only.md b/.changeset/8934-chatter-feed-affordance-only.md index 79a7827f49..4401133fcc 100644 --- a/.changeset/8934-chatter-feed-affordance-only.md +++ b/.changeset/8934-chatter-feed-affordance-only.md @@ -49,10 +49,11 @@ reaches: no-filter. **Not closed by this change**, and not claimed to be: `filterMode` and -`enableMentions` are also members of the declared shape and are still unread on -this path — `RecordActivityTimeline` takes `filterMode` as a component prop -rather than off `config`, and the chatter path's mentions come from the host -context. Tracked as objectui#8968. The host fallback panel described above was +`enableMentions` are also members of the declared shape and were still unread on +this path when this change landed — `RecordActivityTimeline` takes `filterMode` +as a component prop rather than off `config`, and the chatter path's mentions +come from the host context. Tracked as objectui#8968, and closed by it in this +same release — see that entry. The host fallback panel described above was tracked as objectui#8983, since closed — see the note above it. Marked `minor` rather than `patch`: this repository never declares `major` (the diff --git a/.changeset/8968-chatter-filtermode-enablementions.md b/.changeset/8968-chatter-filtermode-enablementions.md new file mode 100644 index 0000000000..0477e2e9aa --- /dev/null +++ b/.changeset/8968-chatter-filtermode-enablementions.md @@ -0,0 +1,32 @@ +--- +'@object-ui/plugin-detail': minor +--- + +`record:chatter` / `record:discussion` now read `feed.filterMode` and +`feed.enableMentions` (objectui#8968). + +`@objectstack/spec` declares `RecordChatterProps.feed` as `RecordActivityProps`, +bound to both block names, so every member of the activity shape is authorable +inside `feed`. objectui#8934 made the four filter members live by running +`applyFeedConfig` on this path; two members sit outside that pipeline and stayed +unread. Both are wired now, with `record:activity`'s own reading rather than a +second local one: + +- **`feed.filterMode`** seeds which slice the panel opens on, normalized through + the same function `record:activity` uses (an unrecognised value opens on `all` + rather than on a filter nothing matches). It seeds component STATE, so the + dropdown stays usable instead of being frozen on the authored value. +- **`feed.enableMentions`** gates the composer's @-autocomplete. `false` + withholds the host discussion context's suggestion list, which is the + behaviour the `record:activity` registration publishes for this key. The + protocol's default is on, so an unauthored member keeps the affordance. + +**Behaviour change for existing schemas.** An authored `feed.filterMode` other +than `all` now actually narrows the chatter feed, and an authored +`feed.enableMentions: false` now actually removes the @-autocomplete. Both were +accepted and discarded before. Schemas that authored neither key are unaffected. + +`filterMode` and `showFilterToggle` are independent: with the dropdown gated off, +an authored `filterMode` becomes the author's fixed slice rather than becoming +inert. The reasoning is in the renderer's docblock and pinned by +`recordChatterFilterModeMentions-8968.test.tsx`. diff --git a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts index d19988edaf..6c0da73302 100644 --- a/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts +++ b/apps/console/src/__tests__/registry-inputs-spec-parity.test.ts @@ -2858,9 +2858,10 @@ const NEWLY_JUDGED_UNPINNED_MEMBERS = [ * `RecordChatterProps.feed` as `RecordActivityProps` (`component.zod.ts:1366`), * so objectui#8934 closed the gap in `renderers/record-chatter.tsx` rather than * narrowing the declaration, and the four filter members are live on that path - * now. (`filterMode` and `enableMentions` are still unread there — - * objectui#8968.) This block's own row is unaffected either way: nothing in - * this file asserts on that description string. + * now. (`filterMode` and `enableMentions` were the remainder; objectui#8968 + * closed them too, so every member of the declared shape is read there now.) + * This block's own row is unaffected either way: nothing in this file asserts + * on that description string. * * ## 41 -> 37, the seventh slice, and the LAST four one-key blocks closed * diff --git a/content/docs/plugins/plugin-detail.mdx b/content/docs/plugins/plugin-detail.mdx index fd2073a909..cf596379a7 100644 --- a/content/docs/plugins/plugin-detail.mdx +++ b/content/docs/plugins/plugin-detail.mdx @@ -333,8 +333,20 @@ decision as a defect is how a warning channel gets ignored: asked for, and an unauthored `limit` is `20`, so a longer feed pages with **Load more** rather than rendering whole. - Two members of the shape are **not** read on this path and authoring them - does nothing here: `filterMode` and `enableMentions` (objectui#8968). + Since objectui#8968 the remaining two members are read here too. + `filterMode` seeds which slice the panel opens on, normalized by the same + function `record:activity` uses, so an unrecognised value opens on `all` + rather than on a filter nothing matches; it seeds component **state**, so the + dropdown stays usable instead of freezing on the authored value. + `enableMentions: false` withholds the composer's @-autocomplete; the + protocol's default is on, so an unauthored member keeps the affordance. + Both values were accepted and discarded before, so a schema that already + authored either one changes behaviour on upgrade — unless what it authored + is what this path already did: `filterMode: 'all'` and + `enableMentions: true` both land on the same feed as before. The case worth + auditing is `showFilterToggle: false` alongside a `filterMode` other than + `all` — that slice is now **pinned** where the panel used to show + everything, and the dropdown that would widen it is not on screen. There is no third path left to disagree with: the host used to auto-append a panel below any page that omitted a discussion block, and objectui#7298 diff --git a/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembers-8071.test.tsx b/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembers-8071.test.tsx index d2c2d9b7e9..8f000cdf66 100644 --- a/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembers-8071.test.tsx +++ b/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembers-8071.test.tsx @@ -64,11 +64,18 @@ * IMPLEMENTATION GAP, and it was closed in the renderer rather than written * into the contract. * - * ⚠️ Two members of the declared shape are still unread on this path and are - * NOT closed by that pipeline: `filterMode` (taken by the timeline as a - * component prop, `:187` / `:212`, never off `config`) and `enableMentions` - * (whose only reads are on the `record:activity` path). Tracked as - * objectui#8968; ⛔ neither is pinned here, in either direction. + * ⚠️ Two members of the declared shape are NOT closed by that pipeline, and + * ⛔ neither is pinned here, in either direction: `filterMode` (which the + * timeline takes as a component PROP — it destructures `filterMode: + * controlledFilter` and resolves `controlledFilter ?? internalFilter`, never + * reading it off `config`) and `enableMentions`. + * + * ⭐ Both were UNREAD on this path when that sentence was first written, and + * that half is no longer true: objectui#8968 wired them into + * `renderers/record-chatter.tsx`, with its own pin in + * `recordChatterFilterModeMentions-8968.test.tsx`. The correction is left in + * place of the old claim rather than removed, because this docblock is the + * record of what each pin does and does not cover. * * ## Resolution * diff --git a/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembersLive-8934.test.tsx b/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembersLive-8934.test.tsx index 4574210a68..ff8174253a 100644 --- a/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembersLive-8934.test.tsx +++ b/packages/plugin-detail/src/renderers/__tests__/recordChatterFeedMembersLive-8934.test.tsx @@ -55,10 +55,17 @@ * `showSubscription && subscription` and `RecordChatterRenderer` passes no * `subscription`, so the component is where its live subject is. * - * ⚠️ NOT covered here, and not claimed: `filterMode` and `enableMentions` are - * also members of the declared shape and are still unread on this path - * (`applyFeedConfig` covers the four filter members only). That gap is - * objectui#8968, and pinning it either way is that card's business. + * ⚠️ NOT covered here, and still not claimed here: `filterMode` and + * `enableMentions` are also members of the declared shape, and `applyFeedConfig` + * covers the four filter members only — so this file asserts nothing about + * either of them, in either direction. + * + * ⭐ What this paragraph used to add, and no longer may: that the two are + * "still unread on this path". They are read now. objectui#8968 wired both into + * `renderers/record-chatter.tsx` and pinned them in + * `recordChatterFilterModeMentions-8968.test.tsx`. The SCOPE sentence above + * survives that card; the claim about the implementation did not, and is + * corrected rather than deleted so the next reader can see which half moved. * * ## Resolution * diff --git a/packages/plugin-detail/src/renderers/__tests__/recordChatterFilterModeMentions-8968.test.tsx b/packages/plugin-detail/src/renderers/__tests__/recordChatterFilterModeMentions-8968.test.tsx new file mode 100644 index 0000000000..33be8bf47d --- /dev/null +++ b/packages/plugin-detail/src/renderers/__tests__/recordChatterFilterModeMentions-8968.test.tsx @@ -0,0 +1,434 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * ══════════════════════════════════════════════════════════════════════════ + * `record:chatter.feed` / `record:discussion.feed` — `filterMode` and + * `enableMentions`, the two members `applyFeedConfig` never reached + * (objectui#8968) + * ══════════════════════════════════════════════════════════════════════════ + * + * objectui#8934 ran the filter PIPELINE on this path, which made `types` / + * `limit` / `showCompleted` / `unifiedTimeline` live. Its own pin file says in + * as many words that it did not reach `filterMode` or `enableMentions`, and + * this file is the other half: the protocol declares + * `RecordChatterProps.feed: RecordActivityProps.optional()` under BOTH names, + * so those two are members of the shape an author may write here, and a + * renderer that reads five of them and drops two has an IMPLEMENTATION GAP + * rather than a narrower contract. + * + * ⛔ The opposite direction — narrowing the ObjectUI type so the declaration + * stops over-promising — is refused by the maintainer's standing principle, + * 原文照录: 我们的项目以 objectstack 协议为准,文档应该以实际实现为准。协议不 + * 正确的应该先修改协议。 The card's own first version pointed that way and + * corrected itself; ⛔ a later edit must not re-derive it. + * + * ## The DECISION pinned here, and what would un-pin it + * + * `filterMode` and `showFilterToggle` are INDEPENDENT members: one names which + * slice the feed shows, the other whether the control that changes it is on + * screen. objectui#8968 left the coupling to the implementer, and this file + * is where that choice became mechanical rather than remembered — the + * `showFilterToggle: false` block below asserts the authored slice STILL + * decides the rows while the dropdown is absent. An edit that makes + * `filterMode` inert when the toggle is off turns that block red, which is the + * whole point of pinning it: the reasoning lives in `../record-chatter`'s + * docblock, and the failure lives here. + * + * ## What makes each leg a reading rather than a green over nothing + * + * KEEP-LEG every filter leg that removes a row is paired with a value + * of the SAME member that keeps it, so "the row is gone" is + * never satisfiable by a panel rendering nothing. + * TRIGGER-VALUE the filter dropdown's own displayed label is asserted + * alongside the rows, so the leg says the authored value + * reached the timeline's CONTROLLED prop — not merely that + * some row set shrank somewhere upstream of it. + * AGREEMENT the same feed and the same authored `filterMode` are + * rendered through `record:activity`, and the two paths must + * produce the same rows. That names WHICH reading runs here: + * a lookalike filter written by hand would disagree with it. + * UNDECLARED-KEY a key the shape does not declare, authored in the same + * position, moves neither member — so the panel is reacting + * to the declared members rather than to the presence of a + * `feed` object. + * + * ## `aria` is NOT pinned here, on purpose + * + * objectui#8968's table filed `aria` as NOT MEASURED. It was measured for this + * branch and the answer put `aria` outside this path's remainder: it is unread + * on `record:activity` too, so it is a `record:*` family gap and not a chatter + * one, and closing it means first deciding how an authored label composes with + * the accessible name the timeline already sets on its own section. The reading + * and that argument are reported on the card. ⚠️ Nothing here re-derives it. + * + * ## Resolution + * + * Nothing resolves through any `dist/`: `../record-chatter`, `../record-activity` + * and `../recordActivityFeed` are this package's own source, and + * `@object-ui/react` is mapped to its `src` by the root `vitest.config.mts` + * alias table. + */ + +import * as React from 'react'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, cleanup, fireEvent, waitFor } from '@testing-library/react'; +import { DiscussionContextProvider } from '@object-ui/react'; +import { ComponentRegistry } from '@object-ui/core'; +import type { FeedItem } from '@object-ui/types'; +import { RecordActivityRenderer } from '../record-activity'; +import '../../index'; + +/** Both registered names are the same renderer on the same `CHATTER_INPUTS`. */ +const BLOCK_NAMES = ['record:chatter', 'record:discussion'] as const; + +/** The composer placeholder in the `en` pack — where `@` autocomplete lives. */ +const COMPOSER = /Leave a comment/; +/** The filter dropdown's accessible name — `showFilterToggle`'s observable. */ +const FILTER_TRIGGER = 'Filter activity'; +/** The `en` labels the dropdown shows for each declared filter mode. */ +const MODE_LABEL = { + all: 'All Activity', + comments_only: 'Comments Only', + changes_only: 'Field Changes', + tasks_only: 'Tasks Only', +} as const; + +/** One mentionable entity, supplied by the host the way app-shell does. */ +const MENTION = { id: 'u1', label: 'Ada Lovelace' }; + +/** + * One row of each kind the filter modes slice on, chronological. Timestamps + * are fixed and well in the past so every row formats through + * `toLocaleDateString()` and two renders are comparable. + */ +const ITEMS: FeedItem[] = [ + { id: 'c-1', type: 'comment', actor: 'Ada', body: 'Root comment', createdAt: '2026-01-02T00:00:00.000Z' }, + { + id: 'f-1', + type: 'field_change', + actor: 'Grace', + createdAt: '2026-01-03T00:00:00.000Z', + fieldChanges: [{ field: 'stage', fieldLabel: 'Stage', oldValue: 'New', newValue: 'Won' }], + }, + { id: 't-1', type: 'task', actor: 'Ada', body: 'Follow up call', createdAt: '2026-01-04T00:00:00.000Z' }, + { id: 'c-2', type: 'comment', actor: 'Grace', body: 'Later comment', createdAt: '2026-01-05T00:00:00.000Z' }, +]; + +/** One text marker per fixture row, so a render reads back as an id list. */ +const MARKERS: Record = { + 'c-1': /Root comment/, + 'f-1': /Stage/, + 't-1': /Follow up call/, + 'c-2': /Later comment/, +}; + +/** + * The members this file is NOT about, all stated, so no leg below rides on a + * default. `showCompleted` stays unstated: its spec default hides the `task` + * row, which is what makes `tasks_only` a meaningful third mode to assert. + */ +const AFFORDANCES = { + showFilterToggle: true, + showCommentInput: true, + enableReactions: false, + enableThreading: false, + showSubscriptionToggle: false, +} as const; + +const handlers = { + onAddComment: vi.fn(), + onAddReply: vi.fn(), + onToggleReaction: vi.fn(), +}; + +/** + * The renderer the REGISTRY resolves for a block name — not the imported + * symbol — so a registration that stopped pointing at this renderer fails here. + */ +function rendererFor(blockName: string): React.ComponentType { + const impl = ComponentRegistry.get(blockName) as React.ComponentType | undefined; + if (!impl) throw new Error(`no renderer registered for ${blockName}`); + return impl; +} + +/** + * The element under test. Split out from `mountAs` so a case can RE-RENDER it: + * every call builds a FRESH `schema` object, which is what makes the "an equal + * authored value must not clobber the user's choice" leg below a real reading + * rather than a test of object identity. + */ +function treeFor( + blockName: string, + feed: unknown, + hostExtra: Record = {}, +): React.ReactElement { + const Renderer = rendererFor(blockName); + const schema: Record = { position: 'bottom' }; + if (feed !== undefined) schema.feed = feed; + return ( + + + + ); +} + +function mountAs( + blockName: string, + feed: unknown, + hostExtra: Record = {}, +) { + return render(treeFor(blockName, feed, hostExtra)); +} + +/** Which fixture rows the panel is currently rendering, in fixture order. */ +function renderedIds(): string[] { + return Object.keys(MARKERS).filter((id) => screen.queryAllByText(MARKERS[id]).length > 0); +} + +/** + * Open the filter dropdown and pick one option, driving the Radix Select the + * way this repo's other suites do — `keyDown` to open, then the option. A leg + * that only asserts the OPENING slice cannot tell a seed from a freeze, so this + * is what makes the "the dropdown stays usable" half of the claim measurable + * instead of merely stated. + */ +async function chooseFilter(optionLabel: string): Promise { + const trigger = screen.getByRole('combobox', { name: FILTER_TRIGGER }); + fireEvent.keyDown(trigger, { key: 'ArrowDown' }); + await waitFor(() => expect(screen.queryAllByRole('option').length).toBeGreaterThan(0)); + fireEvent.click(screen.getByRole('option', { name: optionLabel })); + await waitFor(() => + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(optionLabel), + ); +} + +/** Type `@` into the composer; the suggestion list appears only when fed one. */ +function typeMentionTrigger(): void { + fireEvent.change(screen.getByPlaceholderText(COMPOSER), { target: { value: '@' } }); +} + +beforeEach(() => { + cleanup(); + handlers.onAddComment.mockClear(); + handlers.onAddReply.mockClear(); + handlers.onToggleReaction.mockClear(); +}); + +describe.each(BLOCK_NAMES)('%s: `feed.filterMode` decides the slice the panel opens on (objectui#8968)', (blockName) => { + it('`comments_only` keeps the comments and drops the field change', () => { + mountAs(blockName, { ...AFFORDANCES, filterMode: 'comments_only' }); + expect(renderedIds()).toEqual(['c-1', 'c-2']); + // TRIGGER-VALUE — the dropdown shows the authored mode, so the value + // reached the timeline's controlled prop rather than some private filter. + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.comments_only); + + cleanup(); + mountAs(blockName, { ...AFFORDANCES, filterMode: 'all' }); // KEEP-LEG + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.all); + }); + + it('`changes_only` keeps the field change and drops the comments', () => { + mountAs(blockName, { ...AFFORDANCES, filterMode: 'changes_only' }); + expect(renderedIds()).toEqual(['f-1']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.changes_only); + + cleanup(); + mountAs(blockName, { ...AFFORDANCES, filterMode: 'all' }); // KEEP-LEG + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + }); + + it('`tasks_only` reads the mode and the pipeline both — the task needs `showCompleted` too', () => { + // Two declared members compose here rather than one overriding the other: + // `showCompleted` (the pipeline, objectui#8934) decides whether the + // completed task survives at all, `filterMode` (this card) then picks the + // slice. Without `showCompleted` the tasks slice is legitimately empty. + mountAs(blockName, { ...AFFORDANCES, filterMode: 'tasks_only' }); + expect(renderedIds()).toEqual([]); + + cleanup(); + mountAs(blockName, { ...AFFORDANCES, filterMode: 'tasks_only', showCompleted: true }); + expect(renderedIds()).toEqual(['t-1']); // KEEP-LEG + }); + + it('an unrecognised `filterMode` opens on `all`, never on a slice nothing matches', () => { + // objectui#3151's posture, reached here through the same + // `normalizeFilterMode` `record:activity` uses: an unknown token is skipped + // rather than turned into a predicate that can only ever be empty. + mountAs(blockName, { ...AFFORDANCES, filterMode: 'not_a_mode' }); + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.all); + }); + + it('with no `filterMode` authored the panel opens on `all`', () => { + mountAs(blockName, { ...AFFORDANCES }); + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.all); + }); +}); + +describe.each(BLOCK_NAMES)('%s: ⭐ the DECISION — `filterMode` with `showFilterToggle` off (objectui#8968)', (blockName) => { + it('the authored slice still decides the rows while the dropdown is absent', () => { + mountAs(blockName, { ...AFFORDANCES, showFilterToggle: false, filterMode: 'comments_only' }); + // The control is genuinely off … + expect(screen.queryByRole('combobox', { name: FILTER_TRIGGER })).toBeNull(); + // … and the member the author wrote is still honoured. + expect(renderedIds()).toEqual(['c-1', 'c-2']); + }); + + it('KEEP-LEG — the same toggle-off panel on `all` renders every row', () => { + // Without this leg "the field change is gone" above is satisfiable by a + // panel that renders nothing once its dropdown is removed. + mountAs(blockName, { ...AFFORDANCES, showFilterToggle: false, filterMode: 'all' }); + expect(screen.queryByRole('combobox', { name: FILTER_TRIGGER })).toBeNull(); + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + }); + + it('the toggle is the only thing `showFilterToggle` moves — same rows either way', () => { + // States the independence as an equality rather than as prose: for one + // authored mode, turning the dropdown off changes the CONTROL and nothing + // else. An edit that makes `filterMode` inert without its dropdown breaks + // this equality, not merely a row count. + mountAs(blockName, { ...AFFORDANCES, showFilterToggle: true, filterMode: 'changes_only' }); + const withToggle = renderedIds(); + + cleanup(); + mountAs(blockName, { ...AFFORDANCES, showFilterToggle: false, filterMode: 'changes_only' }); + expect(renderedIds()).toEqual(withToggle); + expect(withToggle).toEqual(['f-1']); // the comparison is not vacuous + }); +}); + +describe.each(BLOCK_NAMES)('%s: the authored value is a SEED, not a freeze (objectui#8968)', (blockName) => { + // ⭐ Why this block exists, stated because it is the one thing the other + // cases here CANNOT catch. Every leg above asserts the slice the panel OPENS + // on, and all of them stay green against a renderer that hands the authored + // value down as a constant — `filterMode={defaultFilterMode}` with no + // `onFilterChange`. That renderer would FREEZE the dropdown: the timeline + // resolves `controlledFilter ?? internalFilter`, so a controlled prop with no + // setter pins the value and swallows every user choice. An assertion set that + // cannot tell working from broken is the shape this card exists to remove, so + // the "stays usable" half is driven here rather than asserted in prose. + + it('⭐ a later user choice moves the rows — the dropdown is live, not pinned', async () => { + mountAs(blockName, { ...AFFORDANCES, filterMode: 'comments_only' }); + expect(renderedIds()).toEqual(['c-1', 'c-2']); + + await chooseFilter(MODE_LABEL.changes_only); + + // The rows follow the user, not the author. Against a frozen dropdown this + // is still ['c-1', 'c-2']. + expect(renderedIds()).toEqual(['f-1']); + }); + + it('an EQUAL authored value re-rendered as a fresh object does not clobber that choice', async () => { + // The resync effect runs on `defaultFilterMode`. That is a normalized + // PRIMITIVE, so a parent re-render carrying a new `feed` object with the + // same authored value compares equal and the effect does not re-fire. + // Without this leg, an effect keyed on the config OBJECT would look correct + // here and would reset the user's choice on every parent render. + const view = render(treeFor(blockName, { ...AFFORDANCES, filterMode: 'comments_only' })); + await chooseFilter(MODE_LABEL.changes_only); + expect(renderedIds()).toEqual(['f-1']); + + view.rerender(treeFor(blockName, { ...AFFORDANCES, filterMode: 'comments_only' })); + expect(renderedIds()).toEqual(['f-1']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.changes_only); + }); + + it('a CHANGED authored value DOES re-seed — the effect earns its place', async () => { + // The other half of the same wire, so "does not clobber" above can never be + // satisfied by an effect that was simply deleted. Re-authoring the member + // has to flow through. + const view = render(treeFor(blockName, { ...AFFORDANCES, filterMode: 'comments_only' })); + await chooseFilter(MODE_LABEL.changes_only); + expect(renderedIds()).toEqual(['f-1']); + + view.rerender(treeFor(blockName, { ...AFFORDANCES, filterMode: 'tasks_only', showCompleted: true })); + expect(renderedIds()).toEqual(['t-1']); + expect(screen.getByRole('combobox', { name: FILTER_TRIGGER }).textContent) + .toContain(MODE_LABEL.tasks_only); + }); +}); + +describe.each(BLOCK_NAMES)('%s: `feed.enableMentions` gates the composer suggestions (objectui#8968)', (blockName) => { + it('unauthored means ON — the host suggestions reach the composer', () => { + // The protocol's default is true, so an author who said nothing must keep + // the affordance. `!== false` rather than truthiness is what makes this so. + mountAs(blockName, { ...AFFORDANCES }, { mentionSuggestions: [MENTION] }); + typeMentionTrigger(); + expect(screen.getByText(MENTION.label)).toBeTruthy(); + }); + + it('`false` withholds them, and the composer itself stays', () => { + mountAs(blockName, { ...AFFORDANCES, enableMentions: false }, { mentionSuggestions: [MENTION] }); + typeMentionTrigger(); + expect(screen.queryByText(MENTION.label)).toBeNull(); + // CONTROL — "no suggestion" is not "no composer", and not "no panel". + expect(screen.getByPlaceholderText(COMPOSER)).toBeTruthy(); + expect(renderedIds()).toEqual(['c-1', 'f-1', 'c-2']); + + cleanup(); + mountAs(blockName, { ...AFFORDANCES, enableMentions: true }, { mentionSuggestions: [MENTION] }); + typeMentionTrigger(); + expect(screen.getByText(MENTION.label)).toBeTruthy(); // KEEP-LEG + }); +}); + +describe('the controls that make the legs above readings (objectui#8968)', () => { + it.each(BLOCK_NAMES)('%s: AGREEMENT — the same authored mode renders the same rows on `record:activity`', (blockName) => { + const mode = 'comments_only'; + + mountAs(blockName, { ...AFFORDANCES, filterMode: mode }); + const chatterRows = renderedIds(); + + cleanup(); + // `record:activity` reads the member off the node root; the chatter path + // reads it off `feed`. Same declared member, one implementation of it. + render(); + const activityRows = renderedIds(); + + expect(activityRows).toEqual(['c-1', 'c-2']); // the comparison is not vacuous + expect(chatterRows).toEqual(activityRows); + }); + + it.each(BLOCK_NAMES)('%s: UNDECLARED-KEY — a key the shape does not declare moves neither member', (blockName) => { + // Without this leg every case above is satisfiable by a panel that reacts + // to the mere presence of an authored `feed` object. + mountAs(blockName, { ...AFFORDANCES }, { mentionSuggestions: [MENTION] }); + const baseline = renderedIds(); + typeMentionTrigger(); + expect(screen.getByText(MENTION.label)).toBeTruthy(); + + cleanup(); + mountAs( + blockName, + { ...AFFORDANCES, notADeclaredMember: 'comments_only', alsoNotDeclared: false }, + { mentionSuggestions: [MENTION] }, + ); + expect(renderedIds()).toEqual(baseline); + typeMentionTrigger(); + expect(screen.getByText(MENTION.label)).toBeTruthy(); + }); + + it('CONTROL — with no host suggestions at all, `enableMentions: true` still offers none', () => { + // Names where the suggestions come from: the member GATES the host's list, + // it does not manufacture one. Otherwise the ON legs above would be + // satisfiable by a composer that invents entries. + mountAs('record:chatter', { ...AFFORDANCES, enableMentions: true }); + typeMentionTrigger(); + expect(screen.queryByText(MENTION.label)).toBeNull(); + expect(screen.getByPlaceholderText(COMPOSER)).toBeTruthy(); + }); +}); diff --git a/packages/plugin-detail/src/renderers/record-chatter.tsx b/packages/plugin-detail/src/renderers/record-chatter.tsx index 1ec652b2f0..6a7eb202e1 100644 --- a/packages/plugin-detail/src/renderers/record-chatter.tsx +++ b/packages/plugin-detail/src/renderers/record-chatter.tsx @@ -13,18 +13,21 @@ * ## `feed` is the full `record:activity` shape, and it is applied HERE * * `@objectstack/spec` declares `RecordChatterProps.feed: - * RecordActivityProps.optional()` (`component.zod.ts:1366`), bound to BOTH - * names (`:2948` `record:chatter`, `:2962` `record:discussion`), and + * RecordActivityProps.optional()` (`ui/component.zod.ts`, the member commented + * `Feed configuration (delegates to RecordActivityProps)`), bound to BOTH names + * — `ComponentPropsMap` wires `'record:chatter'` and `'record:discussion'` to + * that one `RecordChatterProps` row, and the pair-identity pin in + * `component-record-blocks.test.ts` holds them together. And * `RecordActivityProps` carries the filter members `types` / `limit` / * `showCompleted` / `unifiedTimeline`. This renderer used to hand * `discussion.items` to the panel raw, so those four were accepted by the spec, * declared in the registration, and then discarded in silence — an * IMPLEMENTATION GAP, not a narrower contract (objectui#8934). * - * The pipeline below is `record-activity.tsx:219`'s, not a second convention: - * the same `applyFeedConfig(sourceItems, { types, showCompleted, - * unifiedTimeline }, pageSize)` call, and the same reading of `limit` as a PAGE - * SIZE that "Load more" grows by, rather than a hard cap. + * The pipeline below is `record-activity.tsx`'s, not a second convention: the + * same `applyFeedConfig(sourceItems, { types, showCompleted, unifiedTimeline }, + * pageSize)` call, and the same reading of `limit` as a PAGE SIZE that + * "Load more" grows by, rather than a hard cap. * * ⚠️ Consequences an author sees, because the spec's DEFAULTS now apply here * too: `showCompleted` defaults false, so completed activities (feed type @@ -33,19 +36,70 @@ * whole. Both are `record:activity`'s behaviour, which is what the protocol * says this key is. * - * ⚠️ NOT closed by this pipeline, and not claimed to be: `filterMode` and - * `enableMentions` are also declared on `RecordActivityProps` and are still - * unread on this path — `RecordActivityTimeline` takes `filterMode` as a - * component PROP (`:187`, `:212`), never off `config`, and the chatter path's - * mentions come from the host context's `mentionSuggestions`. `applyFeedConfig` - * covers the four filter members only. Tracked as objectui#8968. + * ## `filterMode` and `enableMentions` — the remainder, closed here (objectui#8968) + * + * `applyFeedConfig` is the filter PIPELINE, so it reaches the four members it + * takes and no others. Two more members of the same declared shape sit outside + * it, and both were still unread on this path after objectui#8934: + * + * `filterMode` `RecordActivityTimeline` takes it as a component PROP + * (destructured `filterMode: controlledFilter`, resolved as + * `controlledFilter ?? internalFilter`), never off `config` + * — and this renderer passed none, so an authored + * `feed.filterMode` reached nothing under either name. + * `RecordChatterPanel` already declares and forwards the + * prop; the chatter renderer was the missing end. + * `enableMentions` the composer's @-autocomplete is fed by + * `mentionSuggestions`, which this renderer passed on + * UNCONDITIONALLY — so the affordance existed here and the + * member the protocol says gates it did not. Off now + * withholds the suggestions, which is the behaviour the + * `record:activity` registration publishes for this key. + * + * Both are wired with `record:activity`'s own reading rather than a local one, + * for the reason objectui#8934 gives: one declared shape, one implementation of + * it, not two copies kept in agreement by hand. + * + * ### ⭐ The DECISION this card left to the implementer — `filterMode` on a + * panel whose dropdown is gated off by `showFilterToggle` + * + * `filterMode` and `showFilterToggle` are INDEPENDENT. One names WHICH SLICE + * the feed shows; the other names WHETHER THE CONTROL that changes it is on + * screen. With the toggle off an authored `filterMode` is therefore not inert — + * it stops being the slice the user OPENS on and becomes the slice the author + * PINNED. Three reasons that is the reading, rather than "no dropdown, no + * meaning": + * + * 1. the protocol's own wording splits them exactly that way — `filterMode` + * is described as the "Default activity filter" and `showFilterToggle` as + * "Show filter dropdown in panel header". Neither member's description + * mentions the other, so neither is declared to gate the other. + * 2. `RecordActivityTimeline` already resolves `activeFilter` before, and + * independently of, its `showFilter` flag, and the item filter runs on + * `activeFilter` whether or not the dropdown renders. Coupling the two + * HERE would put a second convention on one declared shape — the defect + * class objectui#8934 closed, re-opened one key along. + * 3. the alternative silently discards a value the author wrote and the spec + * accepts, which is the exact shape this card exists to remove. + * + * Pinned by `__tests__/recordChatterFilterModeMentions-8968.test.tsx`, both + * legs: the pinned slice renders, and the dropdown does not. + * + * ⚠️ `aria` — the third row of objectui#8968's table, which the card filed as + * NOT MEASURED — was measured on that card and is NOT part of this path's + * remainder: it is unread on `record:activity` as well, so it is a gap over the + * `record:*` block family rather than a chatter-path one, and closing it means + * deciding how an authored label composes with the timeline's own. Reported on + * objectui#8968, deliberately not fixed here. ⚠️ Nothing in this file + * re-derives that reading — it was taken once, and the card carries it. */ import React from 'react'; import { useRecordContext, useDiscussionContext } from '@object-ui/react'; import type { FeedItem, RecordActivityComponentProps, RecordChatterComponentProps } from '@object-ui/types'; import { RecordChatterPanel } from '../RecordChatterPanel'; -import { applyFeedConfig, normalizeLimit } from './recordActivityFeed'; +import type { FeedFilterMode } from '../RecordActivityTimeline'; +import { applyFeedConfig, normalizeFilterMode, normalizeLimit } from './recordActivityFeed'; const splitDesigner = (props: Record) => { const { 'data-obj-id': id, 'data-obj-type': type, style, ...rest } = props || {}; @@ -100,19 +154,48 @@ export const RecordChatterRenderer: React.FC = ({ ...(schema as any), } as any; - // The nested `feed` IS `RecordActivityProps` (spec `component.zod.ts:1366`), - // so its filter members are applied on this path with the same pipeline and - // the same call shape `record:activity` uses at `record-activity.tsx:219`. + // The nested `feed` IS `RecordActivityProps` (the spec member commented + // `Feed configuration (delegates to RecordActivityProps)`), so its filter + // members are applied on this path with the same pipeline and the same + // `applyFeedConfig` call shape `record:activity` uses. const feed = config.feed as RecordActivityComponentProps | undefined; // Page window. Starts at `limit` and grows by `limit` per "Load more", so the // authored `limit` is a page size (the spec's wording) rather than a hard cap // — the same reading, and the same `extraPages`-is-the-state shape, as - // `record-activity.tsx:159-160`. + // `record-activity.tsx` (where the comment reads "The extra PAGES are the + // state, not the resulting size"). const [extraPages, setExtraPages] = React.useState(0); const limit = normalizeLimit(feed?.limit); const pageSize = limit * (extraPages + 1); + // `filterMode` — the authored slice, normalized by the SAME function + // `record:activity` uses, so an unrecognised value falls back to `all` rather + // than freezing the dropdown blank on a value with no matching item. + // ⚠️ That shared diagnostic prints a `[record:activity]` prefix and warns + // once per offending VALUE, so a bad `filterMode` authored on both a + // `record:activity` and a `record:chatter` block on one page warns under the + // activity name only. A renderer-aware prefix belongs in `recordActivityFeed` + // rather than here; reported on objectui#8968, not fixed on this card. + // + // It seeds STATE rather than being handed to the panel directly: the timeline + // treats `filterMode` as a CONTROLLED prop, so passing the authored value + // straight through would pin the dropdown open on it and swallow every user + // choice. Same shape as `record:activity`, deliberately — one reading of one + // declared member. + // + // ⛔ Do not "simplify" these three lines into `filterMode={defaultFilterMode}`: + // that is the freeze, and it is invisible to any assertion that only reads the + // slice the panel OPENS on. The seed-not-freeze block in + // `__tests__/recordChatterFilterModeMentions-8968.test.tsx` drives the dropdown + // after mount and turns red when it is done. + // + // ⭐ Independent of `showFilterToggle` on purpose; the docblock above carries + // the decision and its three reasons. + const defaultFilterMode = normalizeFilterMode(feed?.filterMode); + const [filterMode, setFilterMode] = React.useState(defaultFilterMode); + React.useEffect(() => { setFilterMode(defaultFilterMode); }, [defaultFilterMode]); + const discussionItems = discussion?.items as FeedItem[] | undefined; const applied = React.useMemo( () => @@ -128,9 +211,9 @@ export const RecordChatterRenderer: React.FC = ({ [discussionItems, feed?.types, feed?.showCompleted, feed?.unifiedTimeline, pageSize], ); - // `record-activity.tsx:233` writes this with an empty dependency list. The - // setter is named here instead — it is stable, so the two are equivalent at - // runtime — because with `[]` the React Compiler cannot preserve the manual + // `record-activity.tsx` writes its own `handleLoadMore` with an empty + // dependency list. The setter is named here instead — it is stable, so the + // two are equivalent at runtime — because with `[]` the React Compiler cannot preserve the manual // memoization (it infers `setExtraPages`) and reports `Compilation Skipped` // on this component. One deviation, in a dependency array, to keep this // component compiled; the pipeline call shape below is copied unchanged. @@ -138,6 +221,13 @@ export const RecordChatterRenderer: React.FC = ({ setExtraPages((n) => n + 1); }, [setExtraPages]); + // `enableMentions` — withholding the suggestion list is what turns the + // composer's @-autocomplete off, which is what the registration publishes for + // this key ("Off withholds the suggestions"). `!== false` rather than a + // truthiness test, because the protocol's default is ON and an unauthored + // member must not read as "off". + const mentionsEnabled = feed?.enableMentions !== false; + return (
= ({ // is the signal that reaches it). `record:activity` reads the same // field off the same context; no second idiom. loading={discussion?.loading} + // The two members `applyFeedConfig` does not reach (objectui#8968). + // `RecordChatterPanel` already forwards both to the timeline, so the + // renderer was the only missing end of the wire. + filterMode={filterMode} + onFilterChange={setFilterMode} onAddComment={discussion?.onAddComment as any} onAddReply={discussion?.onAddReply as any} onToggleReaction={discussion?.onToggleReaction as any} - mentionSuggestions={discussion?.mentionSuggestions as any} + mentionSuggestions={mentionsEnabled ? (discussion?.mentionSuggestions as any) : undefined} onUploadAttachments={discussion?.onUploadAttachments as any} />