feat(ui): Add typography and table primatives#1
Merged
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAdds a Separator component, a full Table compound component, a complete typography system (types, utils, 12 primitives, CSS, stories, and tests), updates barrel exports and coverage/tests, and extends theme tokens for typography, separators, and tables. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/base-ui/src/components/table/Table.stories.tsx`:
- Around line 112-113: Stories use Link components with href="#" which causes
page jumps in Storybook; update the Table.Cell <Link> usages in
Table.stories.tsx (the Link inside Table.Row / Table.Cell) to either use a
stable absolute URL like "https://example.com" or attach an onClick handler that
calls event.preventDefault() for story-only examples so clicks don't navigate or
jump the page.
In `@packages/base-ui/src/components/table/Table.tsx`:
- Around line 64-70: The resolveAlign function currently forces 'right' when
numeric is true even if an explicit align was provided; modify resolveAlign to
first respect a provided align (i.e., if align is not undefined return align),
otherwise if numeric is true return 'right', else return 'left' so explicit
align overrides the numeric default; update the function named resolveAlign to
implement this precedence.
In `@packages/base-ui/src/components/typography/Em.tsx`:
- Line 27: The ref cast "ref as never" in Em (and other typography components
like Text, Strong, Quote, Overline, Hotkey, Heading, Code, Caption, Blockquote,
Underline) bypasses type safety—replace it with a proper polymorphic ref typing
solution: create or import a polymorphic component/ref helper (e.g., a generic
PolymorphicComponentProps/PolymorphicRef utility) and type the Em component with
generics that map the "as" prop to the correct element ref type, update the
forwardRef signature for Em to use that PolymorphicRef generic, and apply the
same pattern across the other typography components so the ref type matches the
resolved element instead of using "as never".
In `@packages/base-ui/src/components/typography/Heading.stories.tsx`:
- Around line 31-41: The AllLevels story renders fixed Heading text and level
per row, so global controls for children and level should be disabled; update
the AllLevels story configuration (reference AllLevels) to disable those
controls either by adding parameters.controls.exclude: ['children','level'] or
by setting AllLevels.argTypes to disable the children and level controls (e.g.,
argTypes.children.control.disable = true and argTypes.level.control.disable =
true) so the story UI doesn't show misleading controls.
In `@packages/base-ui/src/components/typography/Heading.tsx`:
- Around line 27-38: The Heading component currently allows rendering as a
non-semantic element (e.g., as="div") via the Element variable, which removes
native heading semantics; update the JSX spread so that when Element is a
non-heading (detect when props.as === 'div' or when as is falsy and the computed
tag isn't an h1..h6) you add role="heading" and aria-level={level} to the
rendered element (preserve existing ref, className, data-ov-level, style,
typographyData, truncationData and other props); reference the Element variable
and the HeadingProps['as'] usage to locate where to inject these conditional
attributes.
In `@packages/base-ui/src/components/typography/Overline.tsx`:
- Around line 13-34: The Overline component duplicates a common render pattern
(element selection, class merge, truncation style, and spreading
typography/truncation data); extract that shared wiring into a small internal
helper (e.g., a factory like createTypographyComponent or a primitive wrapper
function) so Overline can call it instead of repeating logic. The helper should
accept parameters for the base styles/class (styles.Overline), default element,
and default props (size/tone/truncate), and internally apply cn,
truncationStyle, typographyData, truncationData, and forwardRef; then replace
the body of Overline to call that helper and pass through Element, className,
style, size, tone, truncate, ref, and ...props to eliminate duplication and keep
behavior identical.
In `@packages/base-ui/src/components/typography/Overview.stories.tsx`:
- Around line 41-42: The sample Link in Overview.stories.tsx uses href="#" which
triggers a hash-jump; update the Link usage (the Link component instance showing
"Open runtime documentation") to prevent navigation by adding an onClick handler
that calls event.preventDefault() (e.g., onClick={(e) => e.preventDefault()}) so
the demo interaction stays in-canvas without changing the page location.
In `@packages/base-ui/src/components/typography/Typography.module.css`:
- Around line 186-191: Stylelint reports a missing empty line before the final
declaration in the .Blockquote[data-ov-variant='plain'] rule; fix it by
inserting a blank line immediately before the border-radius declaration so the
block reads with an empty line separating the previous --_ov-blockquote-bg
declaration and the border-radius property (target the
.Blockquote[data-ov-variant='plain'] selector and the border-radius
declaration).
- Around line 276-283: In the .Link rule (Typography.module.css) change the CSS
keyword from "currentColor" to lowercase "currentcolor" inside the
text-decoration-color color-mix call so it follows the value-keyword-case
convention; update the value in the text-decoration-color property within the
.Link selector to use currentcolor.
In `@packages/base-ui/src/components/typography/utils.ts`:
- Around line 20-22: The truncation branch currently classifies any numeric
truncate > 1 as "multi" but later floors the value (so 1.2 becomes 1), which can
produce a multi-line state with a one-line clamp; update the conditional that
returns { 'data-ov-truncate': 'multi' } to require truncate >= 2 (i.e.,
Number.isFinite(truncate) && truncate >= 2) so only values that will floor to at
least 2 are treated as multi-line, and make the same change in the analogous
block referenced around lines 31–38; adjust the checks that reference the
truncate variable to use the normalized/floored value consistently.
In `@packages/base-ui/src/theme/styles.css`:
- Around line 248-263: The table cell padding tokens
(--ov-size-table-cell-padding-inline-sm/md/lg and
--ov-size-table-cell-padding-block-sm/md/lg) and separator tokens
(--ov-size-separator-thickness-*, --ov-size-separator-inset-*) need
density-specific overrides; add matching declarations under the existing
[data-ov-density='compact'] and [data-ov-density='comfortable'] blocks
(mirroring how button/card spacing is handled) so compact reduces and
comfortable increases these values accordingly, ensuring you edit the density
selector blocks that contain other size tokens and reference the same variable
names to maintain consistency.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: ad3712b9-b1dd-43de-a84b-1bc106ec4aed
📒 Files selected for processing (45)
packages/base-ui/src/components/ComponentCoverage.stories.tsxpackages/base-ui/src/components/base-ui-coverage.test.tspackages/base-ui/src/components/index.tspackages/base-ui/src/components/separator/Separator.module.csspackages/base-ui/src/components/separator/Separator.stories.tsxpackages/base-ui/src/components/separator/Separator.test.tsxpackages/base-ui/src/components/separator/Separator.tsxpackages/base-ui/src/components/separator/index.tspackages/base-ui/src/components/table/Table.module.csspackages/base-ui/src/components/table/Table.stories.tsxpackages/base-ui/src/components/table/Table.test.tsxpackages/base-ui/src/components/table/Table.tsxpackages/base-ui/src/components/table/index.tspackages/base-ui/src/components/typography/Blockquote.stories.tsxpackages/base-ui/src/components/typography/Blockquote.tsxpackages/base-ui/src/components/typography/Caption.stories.tsxpackages/base-ui/src/components/typography/Caption.tsxpackages/base-ui/src/components/typography/Code.stories.tsxpackages/base-ui/src/components/typography/Code.tsxpackages/base-ui/src/components/typography/Em.stories.tsxpackages/base-ui/src/components/typography/Em.tsxpackages/base-ui/src/components/typography/Heading.stories.tsxpackages/base-ui/src/components/typography/Heading.tsxpackages/base-ui/src/components/typography/Hotkey.stories.tsxpackages/base-ui/src/components/typography/Hotkey.tsxpackages/base-ui/src/components/typography/Link.stories.tsxpackages/base-ui/src/components/typography/Link.tsxpackages/base-ui/src/components/typography/Overline.stories.tsxpackages/base-ui/src/components/typography/Overline.tsxpackages/base-ui/src/components/typography/Overview.stories.tsxpackages/base-ui/src/components/typography/Quote.stories.tsxpackages/base-ui/src/components/typography/Quote.tsxpackages/base-ui/src/components/typography/Strong.stories.tsxpackages/base-ui/src/components/typography/Strong.tsxpackages/base-ui/src/components/typography/Text.stories.tsxpackages/base-ui/src/components/typography/Text.tsxpackages/base-ui/src/components/typography/Typography.module.csspackages/base-ui/src/components/typography/Typography.test.tsxpackages/base-ui/src/components/typography/Typography.tsxpackages/base-ui/src/components/typography/Underline.stories.tsxpackages/base-ui/src/components/typography/Underline.tsxpackages/base-ui/src/components/typography/index.tspackages/base-ui/src/components/typography/types.tspackages/base-ui/src/components/typography/utils.tspackages/base-ui/src/theme/styles.css
| ) { | ||
| return ( | ||
| <Element | ||
| ref={ref as never} |
There was a problem hiding this comment.
🧹 Nitpick | 🔵 Trivial
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify where unsafe ref casts are used so the fix can be applied consistently.
rg -nP --type=tsx 'ref=\{ref as never\}' packages/base-ui/src/components/typographyRepository: omniviewdev/ui
Length of output: 85
🏁 Script executed:
cat -n packages/base-ui/src/components/typography/Em.tsxRepository: omniviewdev/ui
Length of output: 1294
🏁 Script executed:
rg -nP 'ref=\{ref as never\}' packages/base-ui/src/components/typographyRepository: omniviewdev/ui
Length of output: 961
🏁 Script executed:
cat -n packages/base-ui/src/components/typography/Text.tsx | head -40Repository: omniviewdev/ui
Length of output: 1498
🏁 Script executed:
# Search for any polymorphic ref patterns or helper functions in the codebase
rg -nP 'forwardRef.*as\?' packages/base-ui/src/components/typography -A 5 | head -100Repository: omniviewdev/ui
Length of output: 40
Avoid ref as never on Line 27; it bypasses ref type safety.
This cast suppresses TypeScript checks for passing a ref to a dynamically determined element (as?: 'em' | 'span'), which can mask type mismatches. However, simply removing the cast will cause TypeScript errors since the ref (typed to HTMLElement) doesn't directly match the union of possible element types.
A proper solution requires polymorphic ref typing—for example, using a type helper that resolves the correct ref type based on the as prop, or restructuring the component to avoid the dynamic element cast altogether. This same issue affects all typography components in this directory (Text, Strong, Quote, Overline, Hotkey, Heading, Code, Caption, Blockquote, Underline) and should be addressed consistently.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@packages/base-ui/src/components/typography/Em.tsx` at line 27, The ref cast
"ref as never" in Em (and other typography components like Text, Strong, Quote,
Overline, Hotkey, Heading, Code, Caption, Blockquote, Underline) bypasses type
safety—replace it with a proper polymorphic ref typing solution: create or
import a polymorphic component/ref helper (e.g., a generic
PolymorphicComponentProps/PolymorphicRef utility) and type the Em component with
generics that map the "as" prop to the correct element ref type, update the
forwardRef signature for Em to use that PolymorphicRef generic, and apply the
same pattern across the other typography components so the ref type matches the
resolved element instead of using "as never".
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
packages/base-ui/src/components/typography/Heading.tsx (1)
55-55: 🧹 Nitpick | 🔵 TrivialConsider allowing explicit
roleoverride for non-semantic elements.Currently, non-semantic elements always receive
role="heading". If a user explicitly passes a differentroleprop, it would be overwritten. Consider preserving the user-provided role:💡 Proposed fix
- role: semanticHeading ? role : 'heading', + role: semanticHeading ? role : (role ?? 'heading'),🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/base-ui/src/components/typography/Heading.tsx` at line 55, The Heading component currently forces role to 'heading' for non-semantic elements and overwrites any user-provided role; update the role assignment in the Heading render/props logic (reference: Heading component, prop name role, variable semanticHeading) so that when semanticHeading is false it preserves an explicit role prop if passed and only defaults to 'heading' when no role was provided (i.e., honor props.role for non-semantic elements, otherwise fallback to 'heading').
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/base-ui/src/components/typography/Typography.module.css`:
- Around line 299-302: The .Link:focus-visible rule removes outline and relies
on box-shadow which is invisible in Windows High Contrast Mode; restore an
outline fallback by adding a transparent outline (e.g., outline: 1px solid
transparent) and optional outline-offset (e.g., outline-offset: 2px) alongside
the existing box-shadow so the focus ring remains visible in High Contrast Mode
while keeping the same visual in normal mode; update the .Link:focus-visible
selector accordingly.
In `@packages/base-ui/src/components/typography/utils.ts`:
- Around line 16-18: The PolymorphicComponentProps type uses a default Props =
{} which triggers the `@typescript-eslint/no-empty-object-type` warning; fix by
either adding an inline eslint disable comment before the type declaration to
suppress the rule or change the default to Props = Record<string, never>
(whichever your codebase prefers). Locate the PolymorphicComponentProps
declaration (it references ElementType and ComponentPropsWithoutRef) and apply
one of these two changes so ESLint no longer flags the empty-object-type.
- Line 99: Add a short inline comment above the forwardRef usage explaining that
forwardRef<any, any> is intentionally used to work around a TypeScript
polymorphic forwardRef limitation and that type safety is re-established by the
subsequent cast (the cast applied to the TypographyPrimitive export), so linters
and future readers understand this pattern and it is not accidental.
---
Duplicate comments:
In `@packages/base-ui/src/components/typography/Heading.tsx`:
- Line 55: The Heading component currently forces role to 'heading' for
non-semantic elements and overwrites any user-provided role; update the role
assignment in the Heading render/props logic (reference: Heading component, prop
name role, variable semanticHeading) so that when semanticHeading is false it
preserves an explicit role prop if passed and only defaults to 'heading' when no
role was provided (i.e., honor props.role for non-semantic elements, otherwise
fallback to 'heading').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
Run ID: 43aad790-4e8f-4317-a3ca-df0383cbf699
📒 Files selected for processing (18)
packages/base-ui/src/components/table/Table.stories.tsxpackages/base-ui/src/components/table/Table.tsxpackages/base-ui/src/components/typography/Blockquote.tsxpackages/base-ui/src/components/typography/Caption.tsxpackages/base-ui/src/components/typography/Code.tsxpackages/base-ui/src/components/typography/Em.tsxpackages/base-ui/src/components/typography/Heading.stories.tsxpackages/base-ui/src/components/typography/Heading.tsxpackages/base-ui/src/components/typography/Hotkey.tsxpackages/base-ui/src/components/typography/Overline.tsxpackages/base-ui/src/components/typography/Overview.stories.tsxpackages/base-ui/src/components/typography/Quote.tsxpackages/base-ui/src/components/typography/Strong.tsxpackages/base-ui/src/components/typography/Text.tsxpackages/base-ui/src/components/typography/Typography.module.csspackages/base-ui/src/components/typography/Underline.tsxpackages/base-ui/src/components/typography/utils.tspackages/base-ui/src/theme/styles.css
This was referenced Mar 7, 2026
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Adding Typography and the simple Table components to the UI library.
Summary by CodeRabbit
New Features
Tests
Documentation