Repository navigation
Conversation
…e's styling props
ReqoreButton rendered `as={Link}` handed the router link every styling
prop it had: `fluid`, `compact`, `flat`, `animate`, `maxWidth`,
`alignSelf`, `tooltip`, the stack-position flags a ControlGroup clones
onto its children, ... and the link wrote them onto its `<a>`. A
resizable, transparent ReqorePanel handed re-resizable `transparent`,
which it spread onto its wrapper div.
Both come from the same gap: a styled component's `shouldForwardProp`
named a few props by hand (Button: fill/color/wrap; Panel: eight), and
a component target receives every prop not named. The hand lists went
stale as props were added.
The styling props of Button, Panel, Tag and Textarea are now listed by
listReqoreStyleProps from a record typed against the component's prop
interface minus what its element takes (TReqoreStylePropKeys), so a new
prop that is not added fails the build instead of leaking. Panel keeps
re-resizable's own props (`size`, `enable`, ...) forwarded.
The sweep of every `as`-capable component found the same leak in Tag
(asBadge, labelKey, size, ...), Paragraph (`_size`, `block`), the
TextEffect base (`block`, `inline`), the Tabs list item (`theme`,
`active`, `size`, ...), the Textarea given Slate's editable (`flat`,
`minimal`, `rounded`, `_size`), the Icon's animated wrapper (`size`,
control-group flags) and the Breadcrumbs item (`interactive`). The
tooltip component no longer hands `tooltip` to any component; the
Timeline item no longer writes `disabled` onto its `li` (it sets
`aria-disabled`), nor the Textarea wrapper `width`/`height` onto its
div.
ReqoreEntityRow, ReqoreSeverityRow, ReqoreCallout, ReqoreFeatureCard and ReqoreTier drew `description` (and ReqoreTestimonial its `quote`) in a ReqoreP, a `<p>`. Consumers pass paragraphs and control groups there, which a `<p>` cannot hold: React warned (validateDOMNesting) and the browser's parser closes the paragraph at the first block, moving the content out of it and its styling. The description is now a ReqoreP rendered as a `div`: the same styled classes, so a plain-text description looks exactly as before. The root of each of these components, the effect's `span`, is rendered as a `div` too, since a span may not hold blocks either. The prop docs say that a description takes block content.
|
📦 Prerelease published for this PR: Install it in a consumer (updates yarn add @qoretechnologies/reqore@0.77.10-pr.709.g1ab099ePublished under the |
qlip visual regression✅ No visual changes. |
…r render DeepScan REACT_INEFFICIENT_PURE_COMPONENT_PROP: the inline style handed to the memoized ReqoreP defeated its memoization. It was there before; the description's element moving to its own lines made DeepScan report it as new.
Member
|
Review: the core fix is right; a few things to do before merging The compiler-checked omit lists, the six descriptions drawn as After #696 lands (it goes in first)
Gaps in the sweep (they existed before, but are cheap to fix here)
Worth deciding
Tests still missing: a textarea in a control group, the description components in a |
This was referenced Oct 7, 2026
Member
Author
|
These commits moved to #708 (one open PR for this work): 3d5c4a3, e0d3bb2 and 0669d75 are cherry-picks of b0eab1d, da78b7a and 1ab099e. 9180338 adapts them to #696 (the panel's media props, the modern Tier's description). This PR's version bump (0.77.10) was dropped; #708 ships as 0.78.1. #708 also carries the library-wide sweep of styling props written as HTML attributes (7f6bd6c). Closing without merging. |
davidnich
added a commit
that referenced
this pull request
Oct 7, 2026
…he card and row surfaces; a state attribute reaches the element a component is rendered as From Foxhoundn's review of #709: - A description that is text (a string or a number) is a `<p>` again, exactly as before, so consumer CSS aimed at `p` still matches it; only other content is drawn in a `div` (getReqoreTextElement). Callout, EntityRow, SeverityRow, FeatureCard, Tier (both appearances) and the Testimonial's quote. - A callout's children are drawn in a `div`: they may be blocks, which the effect's `span` could not hold. - The textarea, and the callout, entity-row, severity-row, feature-card and testimonial surfaces, keep back the flags a ReqoreControlGroup hands its children, and the surfaces their own `disabled`, `radiusSize`, `inheritCustomTheme` (and the callout's `accentPosition`, the callout's and card's `interactive`), which a component they are rendered `as` received. - A component's styling-prop list is built against its default element. Rendered `as` an element that has the state attribute it names (`disabled`, `readOnly`, `checked`, `selected`), the attribute is that element's: `<ReqorePanel as='fieldset' disabled>` disables its fields again, `<ReqoreTag as='button' disabled>` is a disabled button. - COMPONENTS.md: block descriptions, and what reaches the DOM now. Tests: plain text is a `<p>`; callout block children; a textarea in a filling stack group; the five surfaces in a filling stack group and rendered as a router link with every surface prop set; a panel as a fieldset and a tag as a button.
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
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.
Follow-up to #697 (0.77.7): the remaining styling-prop leaks into a component rendered
asanother, and descriptions that could not hold block content.Problems
ReqoreButton as={Link}handed the router link reqore's styling props.<ReqoreControlGroup vertical stack><ReqoreButton as={Link} to='/x' fluid compact /></ReqoreControlGroup>gave the linkfluid,compact,flat,animate,maxWidth,alignSelf,active,readOnly,tooltip,description, the group's stack-position flags (isChild,isFirst,childId, …) and more — 36 props, which the link wrote onto its<a>(React warnings,size="small"attributes). Breadcrumbs and menu items renderedasa link inherit it.resizable+transparentReqorePanelhanded re-resizabletransparent, which it spreads onto its wrapper div.descriptionwas drawn in a<p>inReqoreEntityRow,ReqoreSeverityRow,ReqoreCallout,ReqoreFeatureCardandReqoreTier(and aReqoreTestimonial'squote). Consumers passReqoreP/ReqoreControlGroupthere (qorus-ide: ~14 call sites), which a<p>cannot hold: React warns (validateDOMNesting: <div> cannot appear as a descendant of <p>), and the browser's parser closes the paragraph at the first block, moving the content out of it and out of its styling.Root causes
src/components/Button/index.tsx:341(develop):shouldForwardProp: omitStyleProps('fill', 'color', 'wrap'). A DOM tag still filters throughisPropValid, but a component target receives every prop that is not named, and the button names three.src/components/Panel/index.tsx:587-596: the panel's hand list of eight names lackstransparent(anddisabled,minimal, …). It went stale as props were added — the same failure mode as the button's.src/components/{SeverityRow:365, Callout:423, EntityRow:434, FeatureCard:376}/index.tsx,Tier/index.tsx:154,Testimonial/index.tsx:350: the description is aReqoreP, i.e. a<p>. The root of each of these components is the effect'sspan, which may not hold blocks either.Fixes
src/helpers/styled.ts).TReqoreStylePropKeys<TProps, TTargetProps>is every key of a component's prop interface that is not an attribute/prop of what it renders;listReqoreStyleProps<…>({ … })takes aRecordof exactly those keys. A prop added to the interface and not to the record fails the build instead of leaking, so the lists cannot go stale again.REQORE_BUTTON_STYLE_PROPS— againstIReqoreButtonStyleminus<button>+<a>attributes (disabled,type,href, … still reach the element). The button also keeps backREQORE_CONTROL_GROUP_CHILD_PROPS.REQORE_PANEL_STYLE_PROPS— againstIStyledPanelminus<div>attributes and re-resizable'sResizableProps, so aResizablepanel still receivessize,enable,minWidth, handles.REQORE_TAG_STYLE_PROPS,REQORE_TEXTAREA_STYLE_PROPS(the latter keeps every<textarea>attribute and Slate's ownrenderElement/renderLeaf/decorateforwarded).ReqorePrenderedas='div'. styled-components names classes after the CSS they generate, and thedivgets the same classes as the<p>, so a plain-text description is drawn identically. The root of each of these six components is rendered as adiv(it isdisplay: flex/gridin every case, so that changes nothing visible). Each prop's doc now says that it takes block content.Sweep of every
as-capable component (and everydescription)Each was rendered
asa component that records what it receives:astargetReqoreTagasBadge,labelKey,fixed,minimal,rounded,size,interactive,removable,tooltipREQORE_TAG_STYLE_PROPS+ control-group flagsReqoreP_size,block,tooltip_size+ control-group flagsStyledTextEffect(P, Span, Link, Callout content)block,inlineReqoreTabsListItemtheme,active,activeColor,activeTabMarker,disabled,fixed,intent,padded,size,verticalReqoreTextareaas Slate's editable_size,flat,minimal,roundedREQORE_TEXTAREA_STYLE_PROPSReqoreIconwrapperElement={animated.span}size,customTheme+ control-group flagsReqoreBreadcrumbsIteminteractive(not a button prop)ReqoreTooltipComponenttooltipto every non-DOM componentFound on the way: the Timeline item wrote
disabledonto its<li>(now omitted; the item setsaria-disabled), and the Textarea wrapperwidth/heightonto its<div>.Checked and fine:
ReqoreLink(fixed in #697),ReqoreInput,ReqoreEffect/ReqoreTextEffect, the Menu and DrawerResizable, Message / Notificationanimated.div, Popover'swrapperTag. A panel action'saselement is still handedfixedandcustomTheme— that is the action API (the element is expected to be a reqore control), not a leak.ReqoreButton's andReqoreCheckbox'sdescriptionstay phrasing content: they render inside a<button>/<label>, which may only hold phrasing content.ReqoreEmptyStatealready draws a non-string description in adiv.Not in this PR: many styled
divs across the library still receive the Reqoresizeprop as an HTMLsizeattribute (<div size="normal">). React does not warn about it (it is a valid attribute name) and it changes nothing visible; it is a separate, library-wide sweep.Tests
__tests__/domPropLeaks.test.tsx: 11 new cases — the button as a router link in a vertical stack group (with every styling prop), breadcrumbs and menu items as links, a tag and a paragraph as links in control groups, a transparent panel as a link, a resizable transparent panel, resizable panels in a stack group, a tabs item as a link, a textarea as an editor component and animated icons in a control group; the existing timeline case now also catchesdisabledon its<li>. The stub components mark their element, and the detector also flags a Reqoresizeon it (a valid HTML attribute name, whichisPropValidlets through). Plus negative tests on each style list (it never holdsdisabled/href/type/value/readOnly/ re-resizable'ssize/enable/minWidth…) and a detector self-test. All 20 new tests fail ondevelop.__tests__/blockDescriptions.test.tsx: every component given block content draws it in adiv, keeps it inside the description, has no block inside a<p>or<span>, and logs novalidateDOMNestingwarning; a plain-text description is the container's only content; aReqoreP as='div'gets exactly the classes a<p>does; a detector self-test. The block-content cases fail ondevelop.tsc(test + prod),eslintclean. DeepScan: 0 new issues (the Tier description's inline style, re-reported when its element moved lines, is now a module constant).Visual
Captured locally (headless, one story file at a time) for the 20 affected story files — Button, Panel, Tag, TagGroup, Paragraph, Callout, EntityRow, SeverityRow, FeatureCard, Tier, Testimonial, Timeline, Tabs, TextArea, Icon, Breadcrumbs, Menu, Effect, Span, Link — on
developand on this branch: 354 snapshots, 353 byte-identical. The one that differs,Form Textarea / Form Attributes Mobile, differs in 7 anti-aliased pixels of the focus outline's corner by 1–2 colour levels. Running the branch twice gives both thedevelophash and the branch hash, so it is capture noise, not this change. Plain-text descriptions are pixel-identical, as intended.Version: 0.77.10 (0.77.8 is #696's, 0.77.9 is #708's).
🤖 Generated with Claude Code