Skip to content

feat: depreceate scroll-area component and styled scrollbars - #150

Open
Shrinks99 wants to merge 3 commits into
mainfrom
equality-skill-issue-flareups
Open

Shrinks99 wants to merge 3 commits into
mainfrom
equality-skill-issue-flareups

Conversation

@Shrinks99

Copy link
Copy Markdown
Member

These are left over from the component library conversion and not elements that I want to have stick around! Deprecating in favour of browser-native scroll controls (which we use almost everywhere anyways). Now that we have a skill which gives agents a lot of context on our components, it's becoming more important to clean this stuff up so it doesn't get implemented more widely.

Also adds a changelog going forward for the component library.

@tmccoy14 tmccoy14 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed the scroll-area deprecation across all 3 commits. The removal itself is clean and complete — both CSS entry points updated, no lingering references to the deleted utilities in source or built CSS, and Dialog/Sheet still scroll correctly since .dialog-body and .sheet-body already carry overflow-auto. The docs plumbing works too: llms.txt and the .md twins render the deprecation as intended. lint, format:check, and the full build all pass.

Three things to fix in the CHANGELOG before merging, noted inline.

Comment thread packages/ui/CHANGELOG.md
### Changed

- `DialogBody` and `SheetBody` scroll with the native scrollbar rather than the removed utility class.
- The `equality-design-system` skill tells agents to use native scroll containers and not to restyle scrollbars.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This claims a change that isn't in the branch. grep -rni "scrollbar|scroll area|overflow" packages/skills/ returns nothing — the equality-design-system skill is untouched.

This is the gap CLAUDE.md specifically calls out: the skill is what stops agents in other repos rebuilding what we already ship. Without it, an agent consuming the skill still has no rule against hand-rolling ::-webkit-scrollbar styling. The "Common Mistakes" list in packages/skills/equality-design-system/SKILL.md is the natural home for it.

Either add the guidance to the skill, or drop this line.

Comment thread packages/ui/CHANGELOG.md

### Changed

- `DialogBody` and `SheetBody` scroll with the native scrollbar rather than the removed utility class.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DialogBody and SheetBody aren't exported — there are no such names anywhere in packages/ui/src. The components actually changed are DialogContent and SheetContent (they consume the dialog-body / sheet-body CSS module classes, which is probably where the naming came from).

Comment thread packages/ui/CHANGELOG.md
@@ -0,0 +1,21 @@
# Changelog

Notable changes to Explorer are recorded here, following [Keep a Changelog](https://keepachangelog.com/en/1.1.0/).

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"Explorer" should be "Equality". Worth catching since this file was added to files in package.json, so it now ships to npm and renders on the package page.

@tmccoy14

Copy link
Copy Markdown
Contributor

Heads up on downstream impact: guardian/apps/studio still has live <ScrollArea /> usage in five files.

  • src/components/indicators/apply-indicator-to-control.tsx:209
  • src/components/indicators/indicator-search-list.tsx:90
  • src/components/policies/apply-policy-dialog.tsx:343
  • src/components/controls/control-review-sidebar.tsx:104
  • src/components/controls/shimmers/ControlViewerShimmer.tsx:154

Nothing breaks on merge — studio pins "@eqtylab/equality": "4.0.0" exactly, and it has no usages of the removed styled-vertical-scrollbar / styled-horizontal-scrollbar classes, so the utility removal is a no-op there. The @deprecated JSDoc will start showing up as strikethrough in the editor once someone bumps the pin.

Three of the five are the straightforward h-* + overflow-y-auto swap from the migration guide. Two need a closer look:

  • apply-policy-dialog.tsx:343 uses w-[calc(100%+16px)] pr-4 — that's compensating for the Radix scrollbar's gutter, so the negative-margin trick should come out along with the component rather than be carried over.
  • apply-indicator-to-control.tsx:209 has a bare <ScrollArea> with no height constraint, so it's relying on a flex parent to bound it. Worth confirming it still clips once the Radix viewport wrapper is gone.

Not a blocker for this PR — just flagging so the studio migration gets tracked rather than discovered at the next version bump.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants