Skip to content

chore: Remove fixed dimensions#398

Merged
cb-ekuersch merged 5 commits intocds-v9from
remove-height
Feb 22, 2026
Merged

chore: Remove fixed dimensions#398
cb-ekuersch merged 5 commits intocds-v9from
remove-height

Conversation

@cb-ekuersch
Copy link
Copy Markdown
Contributor

@cb-ekuersch cb-ekuersch commented Feb 16, 2026

What changed? Why?

Removes constant: interactableHeight and updates all usage sites. Components using this constant have now been refactored so that their content dictates their final height.

Removes other instances of hard-coded dimensions (e.g. min-height, border-width) and replaces them with theme vars or eliminates them entirely if the design spec was achievable with content-driven layout/spacing.

In some cases where a value could not be removed (DotCount height), care was taken to ensure the value is at least overridable via props.

Linear:

Root cause (required for bugfixes)

UI changes

iOS Old iOS New
old screenshot new screenshot
Android Old Android New
old screenshot new screenshot
Web Old Web New
old screenshot new screenshot

Testing

How has it been tested?

  • Unit tests
  • Interaction tests
  • Pseudo State tests
  • Manual - Web
  • Manual - Android (Emulator / Device)
  • Manual - iOS (Emulator / Device)

Testing instructions

Illustrations/Icons Checklist

Required if this PR changes files under packages/illustrations/** or packages/icons/**

  • verified visreg changes with Terran (include link to visreg run/approval)
  • all illustration/icons names have been reviewed by Dom and/or Terran

Change management

type=routine
risk=low
impact=sev5

automerge=false

@linear
Copy link
Copy Markdown

linear bot commented Feb 16, 2026

paddingInline:
typeof paddingX === 'number' ? theme.space[paddingX] - borderWidthValue : undefined,
};
}, [borderWidth, theme.borderWidth, theme.space, paddingX, paddingY, style]);
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.

Same thing as above, I could also see someone randomly wanting to set different padding for left and right sides, depending on if they have icons.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

@hcopp these padding values come from the props so they are already ready to be customized. They will just get offset slightly to account for the border, unless you think we should only do that if they DONT provide padding props?

@cb-ekuersch cb-ekuersch force-pushed the remove-height branch 4 times, most recently from 31d958f to 6c24b40 Compare February 18, 2026 22:45
@cb-ekuersch cb-ekuersch requested a review from hcopp February 19, 2026 05:53
@cb-ekuersch cb-ekuersch changed the title chore: Remove fixed heights chore: Remove fixed dimensions Feb 19, 2026
@cb-ekuersch cb-ekuersch force-pushed the remove-height branch 2 times, most recently from 6e13e2b to fbb72e9 Compare February 19, 2026 23:43
import { animationConfig, DefaultSlideButtonHandle } from './DefaultSlideButtonHandle';

export const slideButtonTestID = 'slide-button';
//
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.

nit: could we just add a small description here? Maybe

We are still using a harcoded height for SlideButton due to the background height being arbitrary.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

will do before i merge!

hcopp
hcopp previously approved these changes Feb 20, 2026
Copy link
Copy Markdown
Contributor

@hcopp hcopp left a comment

Choose a reason for hiding this comment

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

Changes look good to me, I don't think Max and Harry have had time to review yet though. Thanks for being so thorough on this change!

haoruikun-cb
haoruikun-cb previously approved these changes Feb 20, 2026
Copy link
Copy Markdown
Contributor

@maximo-macchi-cb maximo-macchi-cb left a comment

Choose a reason for hiding this comment

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

Thanks for the huge effort on this. Great improvement to see coming soon! My comments apply for both web and mobile.

@cb-ekuersch cb-ekuersch dismissed stale reviews from haoruikun-cb and hcopp via 58e6ea9 February 21, 2026 06:56
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 1
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1
CODEOWNERS 🟡 See below

🟡 CODEOWNERS

Code Owner Status Calculation
ui-systems-eng-team 🟡 0/1
Denominator calculation
Additional CODEOWNERS Requirement
Show calculation
Sum 0
0
From CODEOWNERS 1
Sum 1

1 similar comment
@cb-heimdall
Copy link
Copy Markdown
Collaborator

🟡 Heimdall Review Status

Requirement Status More Info
Reviews 🟡 0/1
Denominator calculation
Show calculation
1 if user is bot 0
1 if user is external 0
2 if repo is sensitive 0
From .codeflow.yml 1
Additional review requirements
Show calculation
Max 0
0
From CODEOWNERS 1
Global minimum 0
Max 1
1
1 if commit is unverified 0
Sum 1
CODEOWNERS 🟡 See below

🟡 CODEOWNERS

Code Owner Status Calculation
ui-systems-eng-team 🟡 0/1
Denominator calculation
Additional CODEOWNERS Requirement
Show calculation
Sum 0
0
From CODEOWNERS 1
Sum 1

@cb-ekuersch cb-ekuersch merged commit b48ec52 into cds-v9 Feb 22, 2026
15 checks passed
@cb-ekuersch cb-ekuersch deleted the remove-height branch February 22, 2026 06:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants