Repository navigation
Conversation
Contributor
|
Looks like this PR is not ready to merge, because of the following issues:
Please fix the issues and try again If you have any trouble, please check the PR guidelines |
🦋 Changeset detectedLatest commit: bd7afeb The changes in this PR will be included in the next version bump. This PR includes changesets to release 22 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Contributor
|
Important Draft PR not reviewedDraft PRs are not automatically reviewed by default.
To automatically review draft PRs, update your CodeRabbit configuration: reviews:
auto_review:
drafts: true
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## develop #42974 +/- ##
===========================================
+ Coverage 71.49% 71.97% +0.48%
===========================================
Files 5295 3284 -2011
Lines 228091 183904 -44187
Branches 42119 30214 -11905
===========================================
- Hits 163078 132369 -30709
+ Misses 59794 50146 -9648
+ Partials 5219 1389 -3830
Flags with carried forward coverage won't be shown. Click here to find out more. 🚀 New features to boost your workflow:
|
ggazzo
force-pushed
the
docs/fuselage-component-style-props
branch
from
October 22, 2026 00:35
5e87038 to
143f1dc
Compare
Contributor
🖥️ UI previewBuilt from In Rocket.Chat Desktop with Developer Mode on, open one of these links to load this PR's UI into the workspace in focus:
To pick the workspace, add Or run it in a browser against any server (default npx @rocket.chat/ui-preview 42974 --server https://open.rocket.chat |
CI runs it in the lint job with --max, so the count of styling props passed to Fuselage components cannot grow while they are migrated.
Three of them (rcx-swiper-zoom-*, rcx-sidebar-item__menu, rcx-button--loading) match no CSS rule and are removed. rcx-field-group__item is still needed on CustomFieldsForm's Field, since FieldGroup patches the class onto CustomField, which doesn't forward it; it becomes a className.
AbacPreviewMemberGroup passed marginBlock={8}, which is the margin Divider already has.
Dividers that carried their own block margin or width now sit in containers whose gap and alignment produce the same layout.
…ldGroup styling props
Spacing moves to gap on the container (grouping siblings where only part of them were spaced), ButtonGroup replaces per-button margins and width, and flex-child props already implied by ButtonGroup stretch or the parent's direction are dropped. Every change is pixel-identical.
Inputs that share a FieldRow with a remove button are grouped in a gap Box, so the spacing lands only between them. Every change is pixel-identical.
`flexGrow={1}` repeats the input's own default and `alignItems` lands on the
inner input when there is an addon, so both go away. Margins between a
TextInput and its siblings become `gap` on the row.
FieldRow declares a justify variation that packs its content at the end of the line, for rows that hold a single trailing link.
The thread 'Also send to channel' label, the workspace and onboarding terms labels and the new-user password radio labels keep FieldLabel with its default typography, and the close-chat note becomes a FieldHint. The thread checkbox is inset by a padded Box instead of a Field margin, and the onboarding terms Field is spaced by its FieldGroup (it forwards the className the group patches in) or by its container's gap.
The custom emoji upload button and the canned response preview toggle move out of the FieldLabel into a FieldRow beside it, instead of laying the label out as a flex row.
The OAuth app forms and the app install page center their FieldGroup in a Box carrying the 600px column width, instead of passing the width to the FieldGroup.
…Row justify The add-existing-channels Field is inset by a padded Box instead of its own margin, and the forgot-password links of the login form and the E2EE password modal use FieldRow justify='end'.
The migration leaves one finding, the ContactInfo conflicts Callout, so CI now fails on any new styling prop passed to a Fuselage component.
ggazzo
force-pushed
the
docs/fuselage-component-style-props
branch
from
October 23, 2026 02:19
476f070 to
bff57bf
Compare
ggazzo
changed the title
This branch has not been deployed
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.
Proposed changes (including videos or screenshots)
Fuselage components stop taking
Boxstyling props from their callers. A component accepts DOM/ARIA attributes and the variations it declares (no margins, sizes, flex-child props, colours, freeisorrcx-*props), and the container owns the layout, mostly throughgap. The rule and its reasoning are indocs/proposals/fuselage-component-style-props.md.The pull request has three parts:
The lock (first commit).
packages/fuselage/scripts/scan-style-props.mjslists every use of a Fuselage component that passes a styling prop it doesn't declare, and every element laid out byMargins/Scrollable. The lint job in.github/workflows/ci-code-check.ymlruns it with--max, so the count can only go down. It starts at--max 914, the count ondevelop, and can be merged on its own.The migration. One commit per component or family moves spacing to
gapon the container, sizing and flex-child props to the parent's layout, and spacing that a component should own into its own styles. Fuselage's own compositions get the same treatment.The variations the migration needed. These are declared on the components instead of passed as
Boxprops:StackandPopoverSurface;Dividerflush/light/vertical sizes;Buttoncompact,ButtonGroupstretch='equal',IconButtonbadgePosition/color;Chipsmall/label,Tileflush;TablecolumnsandTableCellrowHeader;Iconcolor(semantic tokens),Skeletoninverted;InputBoxendAddonFlush,FieldRowjustify;Modalsizing,ModalIconvariant.isis narrowed where it is legitimate:Button/IconButtonto'button' | 'a',TableCellto'td' | 'th'.The last two commits lower
--maxto what is left and merge the pull request's changesets into one.developThe one finding left, the ContactInfo conflicts
Callout, stays on purpose: centring its icon would need a newCalloutAPI.Visible changes are small and were reviewed one by one in Storybook. Most layouts are pixel-identical. The intended differences are covered by the visual regression baselines updated in this branch, and are listed in the changeset.
Issue(s)
https://rocketchat.atlassian.net/browse/ARCH-2573
Steps to test or reproduce
node packages/fuselage/scripts/scan-style-props.mjs --max 1passes, and adding a styling prop to any Fuselage component makes it fail.Further comments
Merge order: the first commit (scanner and CI step) can be merged alone first, then the rest.