Skip to content

Commit

Permalink
Consistent box sizing border box (#2115)
Browse files Browse the repository at this point in the history
# Pull Request

## 🀨 Rationale

Switch to using `box-sizing: border-box;` consistently in all our
components.

## πŸ‘©β€πŸ’» Implementation

- Each component should use the `display` helper when defined which was
made consistent in #2114. This PR leverages that helper to make sure
each component sets the elements in the shadow root to use `box-sizing:
border-box;` consistently
- Created separate selectors for targeting all children, i.e. `* {}`,
the host itself, i.e. `:host {}`, and all before and after
pseduo-elements so devtools stays clearer, specifically the selector for
`:host` and the selector for `*` are clear in dev tools without the
noise of the `::before` and `::after` selectors
- Cleared the manual configuration for `box-sizing`
- Cleaned up some padding configurations (now consistently included in
the size inside the border) to be margin configurations (which are
outside the border and not part of the size).

## πŸ§ͺ Testing

Rely on chromatic, explanation of some of the changes:
- Menu changes look correct, the minimum size now includes the border
instead of the border going outside the configured size
- Select filter box changes look improved. The sizing of the filter box
and the no options box now fit the control height instead of being
larger than the control height
- Rich text changes look innocuous. Added the interaction design figma
we have but don't think there are strict visual design docs to
reference.

## βœ… Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed.
  • Loading branch information
rajsite authored Jun 10, 2024
1 parent 96e1725 commit db8b50e
Show file tree
Hide file tree
Showing 40 changed files with 30 additions and 47 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Switch to custom box-sizing: border-box consistently",
"packageName": "@ni/nimble-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Switch to custom box-sizing: border-box consistently",
"packageName": "@ni/spright-components",
"email": "[email protected]",
"dependentChangeType": "patch"
}
1 change: 0 additions & 1 deletion packages/nimble-components/src/anchor-menu-item/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,6 @@ export const styles = css`
display: grid;
contain: layout;
overflow: visible;
box-sizing: border-box;
height: ${controlHeight};
grid-template-columns: 1fr;
column-gap: 8px;
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/anchor-tab/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export const styles = css`
:host {
position: relative;
box-sizing: border-box;
font: ${buttonLabelFont};
height: ${controlHeight};
color: ${bodyFontColor};
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/anchor-tabs/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export const styles = css`
${display('grid')}
:host {
box-sizing: border-box;
grid-template-columns: auto 1fr;
grid-template-rows: auto 1fr;
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/anchor-tree-item/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,6 @@ export const styles = css`
.positioning-region {
display: flex;
position: relative;
box-sizing: border-box;
height: calc(${iconSize} * 2);
width: 100%;
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/anchor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const styles = css`
${display('inline')}
:host {
box-sizing: border-box;
font: ${linkFont};
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/breadcrumb-item/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export const styles = css`
:host {
height: ${controlHeight};
box-sizing: border-box;
padding-left: calc(4px - ${borderWidth});
${
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/breadcrumb/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ export const styles = css`
${display('inline-block')}
:host {
box-sizing: border-box;
font: ${linkFont};
--ni-private-breadcrumb-link-font-color: ${linkFontColor};
--ni-private-breadcrumb-link-active-font-color: ${linkActiveFontColor};
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/card-button/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export const styles = css`
cursor: pointer;
outline: none;
border: none;
box-sizing: border-box;
transition: box-shadow ${smallDelay};
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/checkbox/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const styles = css`
.control {
width: calc(${controlHeight} / 2);
height: calc(${controlHeight} / 2);
box-sizing: border-box;
flex-shrink: 0;
border: ${borderWidth} solid ${borderColor};
padding: 2px;
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/combobox/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ export const styles = css`
border-right: 2px solid rgba(${borderRgbPartialColor}, 0.15);
height: calc(${controlHeight} - 12px);
align-self: center;
padding-left: 4px;
margin-left: 4px;
}
.dropdown-button {
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/drawer/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ export const styles = css`
}
.dialog-contents {
box-sizing: border-box;
display: flex;
flex-direction: column;
position: absolute;
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/menu-item/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export const styles = css`
:host {
contain: layout;
overflow: visible;
box-sizing: border-box;
height: ${controlHeight};
grid-template-columns: 1fr;
column-gap: 8px;
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/menu/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ export const styles = css`
}
::slotted(hr) {
box-sizing: content-box;
box-sizing: border-box;
height: 2px;
margin: ${smallPadding};
border: none;
Expand Down
3 changes: 1 addition & 2 deletions packages/nimble-components/src/number-field/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,6 @@ export const styles = css`
}
.root {
box-sizing: border-box;
position: relative;
display: flex;
flex-direction: row;
Expand Down Expand Up @@ -170,7 +169,7 @@ export const styles = css`
.error-icon {
order: 1;
padding-right: ${smallPadding};
margin-right: ${smallPadding};
}
`.withBehaviors(
appearanceBehavior(
Expand Down
5 changes: 1 addition & 4 deletions packages/nimble-components/src/patterns/button/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ export const styles = css`
cursor: pointer;
outline: none;
border: none;
box-sizing: border-box;
${
/*
Not sure why but this is needed to get buttons with icons and buttons
Expand All @@ -55,7 +54,6 @@ export const styles = css`
height: 100%;
width: 100%;
border: ${borderWidth} solid transparent;
box-sizing: border-box;
color: inherit;
border-radius: inherit;
fill: inherit;
Expand Down Expand Up @@ -89,10 +87,9 @@ export const styles = css`
width: 100%;
height: 100%;
pointer-events: none;
box-sizing: border-box;
outline: 0px solid transparent;
color: transparent;
background-clip: content-box;
background-clip: border-box;
transition: outline ${smallDelay} ease-in-out;
}
Expand Down
3 changes: 0 additions & 3 deletions packages/nimble-components/src/patterns/dropdown/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ export const styles = css`
${display('inline-flex')}
:host {
box-sizing: border-box;
color: ${bodyFontColor};
font: ${bodyFont};
height: ${controlHeight};
Expand Down Expand Up @@ -109,7 +108,6 @@ export const styles = css`
.control {
align-items: center;
box-sizing: border-box;
cursor: pointer;
display: flex;
min-height: 100%;
Expand All @@ -136,7 +134,6 @@ export const styles = css`
}
.listbox {
box-sizing: border-box;
display: inline-flex;
flex-direction: column;
overflow-y: auto;
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/radio/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ export const styles = css`
position: relative;
width: calc(${controlHeight} / 2);
height: calc(${controlHeight} / 2);
box-sizing: border-box;
flex-shrink: 0;
border: ${borderWidth} solid ${borderColor};
border-radius: 100%;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const styles = css`
${display('inline-block')}
:host {
box-sizing: border-box;
font: ${mentionFont};
}
Expand Down
3 changes: 0 additions & 3 deletions packages/nimble-components/src/rich-text/editor/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,6 @@ export const styles = css`
}
.container {
box-sizing: border-box;
display: flex;
flex-direction: column;
position: relative;
Expand Down Expand Up @@ -132,7 +131,6 @@ export const styles = css`
/* This padding ensures that showing/hiding the error icon doesn't affect text layout */ ''
}
padding-right: calc(${iconSize});
box-sizing: border-box;
position: relative;
color: inherit;
}
Expand Down Expand Up @@ -258,7 +256,6 @@ export const styles = css`
${toolbarTag}::part(positioning-region) {
background: transparent;
padding-right: 8px;
box-sizing: border-box;
gap: 0px;
height: var(--ni-private-rich-text-editor-footer-section-height);
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/rich-text/viewer/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ export const styles = css`
.viewer {
font: inherit;
outline: none;
box-sizing: border-box;
position: relative;
color: inherit;
overflow-wrap: anywhere;
Expand Down
2 changes: 0 additions & 2 deletions packages/nimble-components/src/switch/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ export const styles = css`
display: flex;
height: var(--ni-private-switch-height);
width: calc(var(--ni-private-switch-height) * 2);
box-sizing: border-box;
background-color: ${fillHoverColor};
border-radius: calc(var(--ni-private-switch-height) / 2);
align-items: center;
Expand Down Expand Up @@ -101,7 +100,6 @@ export const styles = css`
justify-content: center;
align-items: center;
background-color: var(--ni-private-switch-indicator-background-color);
box-sizing: border-box;
width: var(--ni-private-switch-indicator-size);
height: var(--ni-private-switch-indicator-size);
border-radius: calc(var(--ni-private-switch-indicator-size) / 2);
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tab-panel/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ export const styles = css`
${display('block')}
:host {
box-sizing: border-box;
font: ${bodyFont};
color: ${bodyFontColor};
padding-top: ${standardPadding};
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tab/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ export const styles = css`
:host {
position: relative;
box-sizing: border-box;
font: ${buttonLabelFont};
height: ${controlHeight};
color: ${bodyFontColor};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@ export const styles = css`
align-items: center;
height: calc(${controlHeight} + 2 * ${borderWidth});
border-top: calc(2 * ${borderWidth}) solid ${applicationBackgroundColor};
box-sizing: border-box;
grid-template-columns:
calc(
${controlHeight} *
Expand Down
2 changes: 0 additions & 2 deletions packages/nimble-components/src/table/components/row/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@ export const styles = css`
background-color: ${applicationBackgroundColor};
height: calc(${controlHeight} + 2 * ${borderWidth});
border-top: calc(2 * ${borderWidth}) solid transparent;
box-sizing: border-box;
background-clip: padding-box;
}
Expand All @@ -36,7 +35,6 @@ export const styles = css`
width: 100%;
height: ${controlHeight};
pointer-events: none;
box-sizing: border-box;
bottom: 0px;
position: absolute;
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tabs-toolbar/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ export const styles = css`
:host {
align-items: center;
height: ${controlHeight};
box-sizing: border-box;
font: ${bodyFont};
color: ${bodyFontColor};
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tabs/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ export const styles = css`
${display('grid')}
:host {
box-sizing: border-box;
grid-template-columns: auto 1fr;
grid-template-rows: auto 1fr;
}
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/text-area/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -92,7 +92,6 @@ export const styles = css`
font: inherit;
flex-grow: 1;
outline: none;
box-sizing: border-box;
position: relative;
color: inherit;
border-radius: 0px;
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/text-field/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,6 @@ export const styles = css`
}
.root {
box-sizing: border-box;
position: relative;
display: flex;
flex-direction: row;
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tooltip/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@ export const styles = css`
}
.tooltip {
box-sizing: border-box;
flex-shrink: 0;
max-width: 440px;
box-shadow: ${elevation2BoxShadow};
Expand Down
1 change: 0 additions & 1 deletion packages/nimble-components/src/tree-item/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,6 @@ export const styles = css`
.positioning-region {
display: flex;
position: relative;
box-sizing: border-box;
height: calc(${iconSize} * 2);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/utilities/style/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ import {
*/
export const display: typeof foundationDisplay = (
displayValue: CSSDisplayPropertyValue
) => `${foundationDisplay(displayValue)}`;
) => `${foundationDisplay(displayValue)}:host{box-sizing:border-box;}*{box-sizing:border-box;}:host::before,:host::after,::before,::after{box-sizing:border-box;}`;
2 changes: 1 addition & 1 deletion packages/spright-components/src/utilities/style/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,4 +10,4 @@ import {
*/
export const display: typeof foundationDisplay = (
displayValue: CSSDisplayPropertyValue
) => `${foundationDisplay(displayValue)}`;
) => `${foundationDisplay(displayValue)}:host{box-sizing:border-box;}*{box-sizing:border-box;}:host::before,:host::after,::before,::after{box-sizing:border-box;}`;
2 changes: 1 addition & 1 deletion packages/storybook/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -105,5 +105,5 @@ configureActions({
depth: 1
});

// Update the GUID on this line to trigger a turbosnap full rebuild: 835ca179-52c1-4c5c-87c7-f2989f63ec9c
// Update the GUID on this line to trigger a turbosnap full rebuild: 48e02ae5-fcf1-48ed-8108-b7a1f54b7b3b
// See https://www.chromatic.com/docs/turbosnap/#full-rebuilds
6 changes: 6 additions & 0 deletions packages/storybook/src/docs/component-status.stories.ts
Original file line number Diff line number Diff line change
Expand Up @@ -316,6 +316,9 @@ const components = [
{
componentName: 'Rich Text Editor',
componentHref: './?path=/docs/incubating-rich-text-editor--docs',
designHref:
'https://www.figma.com/design/Q5SU1OwrnD08keon3zObRX/SystemLink-orig?node-id=6280-94045&m=dev',
designLabel: 'Figma',
issueHref: 'https://github.com/ni/nimble/issues/1288',
issueLabel: 'Issue',
componentStatus: ComponentFrameworkStatus.incubating,
Expand All @@ -325,6 +328,9 @@ const components = [
{
componentName: 'Rich Text Viewer',
componentHref: './?path=/docs/incubating-rich-text-viewer--docs',
designHref:
'https://www.figma.com/design/Q5SU1OwrnD08keon3zObRX/SystemLink-orig?node-id=6280-94045&m=dev',
designLabel: 'Figma',
issueHref: 'https://github.com/ni/nimble/issues/1288',
issueLabel: 'Issue',
componentStatus: ComponentFrameworkStatus.incubating,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,7 @@ const component = (
[appearanceName, appearance]: AppearanceState
): ViewTemplate => html`
<${numberFieldTag}
style="width: 250px; padding: 8px;"
style="width: 250px; margin: 8px;"
value="${() => valueValue}"
placeholder="${() => placeholderValue}"
appearance="${() => appearance}"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ export const richTextEditorSlotButtons: StoryFn = createStory(html`

// prettier-ignore
const mobileWidthComponent = html`
<${richTextEditorTag} style="padding: 20px; width: 360px; height: 250px;">
<${richTextEditorTag} style="margin: 20px; width: 360px; height: 250px;">
<${richTextMentionUsersTag} pattern="^user:(.*)">
<${mappingUserTag} key="user:1" display-name="John Doe"></${mappingUserTag}>
</${richTextMentionUsersTag}>
Expand Down
Loading

0 comments on commit db8b50e

Please sign in to comment.