mirror of
https://github.com/simstudioai/sim.git
synced 2026-09-24 15:45:35 +08:00
fix(rich-markdown-editor): fix mention chip losing ambient color inside links/h6 (#5594)
* fix(rich-markdown-editor): fix mention chip losing ambient color (same class as #5573) Auditing the whole "an element's own explicit color always wins over an inherited one" bug class (previously fixed for strong/em/code/del/s vs. links and h6 in #5573) turned up one more instance: the @-mention chip's label hardcoded text-[var(--text-primary)], which is redundant with the prose default anyway (matching the strong/em/code precedent) and silently overrides any ambient color a mention's container legitimately sets — a link's blue, or h6's dimmer --text-secondary — since a mention is inline content that can appear inside either (e.g. "###### see @some-file"). Removed the hardcoded color entirely so the label inherits correctly in every context, same fix as strong/em/code. The icon's own monochrome --text-icon fallback is untouched (icons intentionally don't follow ambient text color). New test renders MentionChipView directly and asserts the wrapper carries no explicit text-color utility class; verified it fails against the pre-fix className. * fix(rich-markdown-editor): broaden mention-chip color-regression guard beyond the exact old class Greptile: the test only matched the literal old text-[var(--text-primary)] string — a future edit swapping it for e.g. text-[var(--text-secondary)] or text-blue-500 would still silently reintroduce the ambient-color bug and pass this test. Now checks every non-descendant-scoped (excludes the [&>svg]: icon rule) text-* utility on the wrapper against a color-shaped pattern (arbitrary value, color-shade pairs, or a named color keyword), so any bare text color slipping back in fails. Verified against a text-blue-500 regression. * fix(rich-markdown-editor): close the semantic-Tailwind-color gap in the mention-chip test Greptile: the color-shaped regex still missed semantic theme tokens (text-primary, text-muted-foreground, text-chart-1, etc.) since they don't match a shade-suffix or bracket pattern. Rather than keep enumerating Tailwind's color-naming schemes, flag ANY unscoped text-* utility on the wrapper — none is legitimate on this chip today, so this can only be a color slipping back in. Verified against text-primary/text-muted-foreground/text-chart-1 regressions. * fix(rich-markdown-editor): catch Tailwind's self-targeting [&]:text-* variant too Greptile: the previous filter excluded ANY class starting with `[&`, which also dropped Tailwind's self-targeting arbitrary variant (`[&]:text-primary` applies to the element itself, same as a bare `text-primary`) — only descendant variants like `[&>svg]:text-*` should be excluded. Now explicitly catches both the bare and `[&]:` forms. Verified against a `[&]:text-primary` regression.
This commit is contained in:
+81
@@ -0,0 +1,81 @@
|
||||
/**
|
||||
* @vitest-environment jsdom
|
||||
*
|
||||
* The chip label must never carry its own explicit text color — see the comment on `CHIP_CLASS` in
|
||||
* `mention-chip.tsx`. An element's own explicit `color` always wins over an inherited one regardless
|
||||
* of ancestor specificity, so hardcoding a color here would silently override any ambient color a
|
||||
* mention's container legitimately sets (a link's blue, an `h6` heading's dimmer `--text-secondary`) —
|
||||
* the same bug class already fixed for `strong`/`em`/`code` in `rich-markdown-editor.css`.
|
||||
*/
|
||||
import { act } from 'react'
|
||||
import type { Editor } from '@tiptap/react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import { afterEach, describe, expect, it, vi } from 'vitest'
|
||||
|
||||
vi.mock('next/navigation', () => ({
|
||||
useRouter: () => ({ push: vi.fn() }),
|
||||
useParams: () => ({}),
|
||||
}))
|
||||
|
||||
// Override the global `getAllBlocks: () => ({})` stub — `getIconColorMap` iterates it as an array.
|
||||
vi.mock('@/blocks/registry', () => ({
|
||||
getAllBlocks: () => [],
|
||||
}))
|
||||
|
||||
const { MentionChipView } = await import('./mention-chip')
|
||||
|
||||
function fakeNode(attrs: Record<string, unknown>) {
|
||||
return { attrs } as unknown as Parameters<typeof MentionChipView>[0]['node']
|
||||
}
|
||||
|
||||
function fakeEditor(): Editor {
|
||||
return { storage: { mention: { navigable: false } } } as unknown as Editor
|
||||
}
|
||||
|
||||
let container: HTMLDivElement | null = null
|
||||
let root: Root | null = null
|
||||
|
||||
afterEach(() => {
|
||||
if (root) act(() => root?.unmount())
|
||||
container?.remove()
|
||||
container = null
|
||||
root = null
|
||||
})
|
||||
|
||||
describe('MentionChipView', () => {
|
||||
it('renders its wrapper with no explicit text-color utility class', async () => {
|
||||
;(globalThis as { IS_REACT_ACT_ENVIRONMENT?: boolean }).IS_REACT_ACT_ENVIRONMENT = true
|
||||
container = document.createElement('div')
|
||||
document.body.appendChild(container)
|
||||
root = createRoot(container)
|
||||
|
||||
await act(async () => {
|
||||
root?.render(
|
||||
MentionChipView({
|
||||
node: fakeNode({ kind: 'file', id: 'f1', label: 'notes.md' }),
|
||||
editor: fakeEditor(),
|
||||
} as Parameters<typeof MentionChipView>[0])
|
||||
)
|
||||
})
|
||||
|
||||
const chip = container.querySelector('.mention-chip') as HTMLElement
|
||||
expect(chip).not.toBeNull()
|
||||
|
||||
// Any `text-*` utility targeting the wrapper itself — bare, or Tailwind's self-targeting
|
||||
// `[&]:text-*` arbitrary variant (as opposed to a descendant variant like `[&>svg]:text-*`,
|
||||
// which the icon rule below legitimately uses) — would regress this fix, not just the specific
|
||||
// old `text-[var(--text-primary)]` class. Rather than enumerate every Tailwind color-naming
|
||||
// scheme (arbitrary value, shade-suffixed, semantic theme tokens like `text-primary`/
|
||||
// `text-muted-foreground`, keywords), flag ANY such token: none is legitimate on this wrapper
|
||||
// today, so this can only ever be a color utility slipping back in. A genuinely new, non-color
|
||||
// `text-*` need (e.g. a font-size utility) should fail this test and force an explicit update,
|
||||
// not be silently allowed through.
|
||||
const ownTextUtilities = chip.className
|
||||
.split(/\s+/)
|
||||
.filter((cls) => cls.startsWith('text-') || cls.startsWith('[&]:text-'))
|
||||
expect(ownTextUtilities).toEqual([])
|
||||
|
||||
// The icon's own monochrome fallback is unrelated and must be untouched by this fix.
|
||||
expect(chip.className).toContain('[&>svg]:text-[var(--text-icon)]')
|
||||
})
|
||||
})
|
||||
+9
-2
@@ -15,9 +15,16 @@ import { simLinkPath } from './sim-link'
|
||||
* color, and a 12px icon. Integration icons keep their brand color via
|
||||
* {@link getBareIconStyle} (see {@link MentionChipView}); other kinds stay
|
||||
* monochrome through the `--text-icon` fallback below.
|
||||
*
|
||||
* No explicit label color — an element's own explicit `color` always wins over an inherited one
|
||||
* regardless of ancestor specificity, so hardcoding `--text-primary` here (redundant with the prose
|
||||
* default anyway) would silently override any ambient color a ancestor legitimately sets — a link's
|
||||
* blue, or `h6`'s dimmer `--text-secondary` — since a mention is inline content and can appear inside
|
||||
* either. Omitting it lets the label inherit correctly in both cases, same fix as `strong`/`em`/`code`
|
||||
* in rich-markdown-editor.css.
|
||||
*/
|
||||
const CHIP_CLASS =
|
||||
'mention-chip mx-px inline-flex items-center gap-1 align-middle text-[var(--text-primary)] leading-[1.5] [&>svg]:size-[12px] [&>svg]:shrink-0 [&>svg]:text-[var(--text-icon)]'
|
||||
'mention-chip mx-px inline-flex items-center gap-1 align-middle leading-[1.5] [&>svg]:size-[12px] [&>svg]:shrink-0 [&>svg]:text-[var(--text-icon)]'
|
||||
|
||||
/**
|
||||
* Live chip: an entity icon + label matching the chat input's mention rendering. Where the host opted
|
||||
@@ -25,7 +32,7 @@ const CHIP_CLASS =
|
||||
* inert so a click can't navigate away from an unsaved edit. This view pulls the block registry (for
|
||||
* integration brand icons), so it's kept out of the headless {@link MarkdownMention} module.
|
||||
*/
|
||||
function MentionChipView({ node, editor }: ReactNodeViewProps) {
|
||||
export function MentionChipView({ node, editor }: ReactNodeViewProps) {
|
||||
const router = useRouter()
|
||||
const params = useParams()
|
||||
const { kind, id, label } = node.attrs as MentionAttrs
|
||||
|
||||
Reference in New Issue
Block a user