Skip to content

Commit

Permalink
Use display helper consistently for all components (#2114)
Browse files Browse the repository at this point in the history
# Pull Request

## 🤨 Rationale

This PR updates all defined components to use a custom version of the
`display` helper. The goal of this PR is to be a refactor to use the
same helper but should be effectively a no-op.

Part of #1326

## 👩‍💻 Implementation

- Added a custom display helper
- Updated all elements to use it
- Updates elements that were missing it to have one with a value that
reflects what shows up in storybook today as the computed display value
- Utility elements that should never be visible / have visible content
were set to `display: none;`

## 🧪 Testing

Rely on storybook to verify nothing changed.

## ✅ Checklist

- [x] I have updated the project documentation to reflect my changes or
determined no changes are needed. Updated the `display` doc topic.
  • Loading branch information
rajsite authored May 16, 2024
1 parent 7915e0b commit 17ddea5
Show file tree
Hide file tree
Showing 79 changed files with 192 additions and 77 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Switch to custom display helper",
"packageName": "@ni/nimble-components",
"email": "rajsite@users.noreply.github.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Switch to custom display helper",
"packageName": "@ni/spright-components",
"email": "rajsite@users.noreply.github.com",
"dependentChangeType": "patch"
}
6 changes: 6 additions & 0 deletions packages/eslint-config-nimble/typescript.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ const restrictedImportsPaths = () => [
message:
'Please use focusVisible from src/utilities/style/focus instead.'
},
{
name: '@microsoft/fast-foundation',
importNames: ['display'],
message:
'Please use display from src/utilities/style/display instead.'
},
{
name: '@microsoft/fast-components',
message:
Expand Down
6 changes: 3 additions & 3 deletions packages/nimble-components/docs/css-guidelines.md
Original file line number Diff line number Diff line change
Expand Up @@ -198,13 +198,13 @@ When styling the invalid state of a form component, it may seem natural to use `

Instead of styling based on `:invalid`, style the `[error-visible]` attribute. Then the client can create a binding to apply the `invalid` class based on the associated `FormControl`'s status properties, like `invalid`, `dirty`, and `touched`.

## Use FAST's display utility for styling host element
## Use the display utility for styling host element

For consistent styling, use FAST's `display` utility when setting a `display` style on the host element.
For consistent styling, use the `display` utility when setting a `display` style on the host element.

```ts
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';

export const styles = css`
${display('flex')}
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/anchor-menu-item/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';

import {
bodyDisabledFontColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/anchor-tab/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';
import {
bodyDisabledFontColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/anchor-tabs/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';

export const styles = css`
${display('grid')}
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/anchor-tree-item/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
bodyFontFamily,
bodyFontWeight,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/anchor/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';
import {
linkActiveFontColor,
Expand Down
7 changes: 5 additions & 2 deletions packages/nimble-components/src/anchored-region/styles.ts
Original file line number Diff line number Diff line change
@@ -1,15 +1,18 @@
import { css } from '@microsoft/fast-element';
import { ZIndexLevels } from '../utilities/style/types';
import { display } from '../utilities/style/display';

export const styles = css`
${display('block')}
:host {
/* Avoid using the 'display' helper to customize hidden behavior */
display: block;
contain: layout;
z-index: ${ZIndexLevels.zIndex1000};
}
${'' /* Override 'display' helper hidden behavior */}
:host([hidden]) {
display: block;
visibility: hidden;
}
`;
2 changes: 1 addition & 1 deletion packages/nimble-components/src/banner/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import {
BannerFail100DarkUi,
Black75,
Expand All @@ -10,6 +9,7 @@ import {
Warning100LightUi,
White
} from '@ni/nimble-tokens/dist/styledictionary/js/tokens';
import { display } from '../utilities/style/display';

import {
actionRgbPartialColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/breadcrumb-item/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
bodyFontColor,
borderHoverColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/breadcrumb/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
bodyEmphasizedFont,
linkActiveFontColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/card-button/styles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import {
Black91,
White
} from '@ni/nimble-tokens/dist/styledictionary/js/tokens';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';

import {
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/card/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
bodyEmphasizedFontWeight,
bodyFont,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/checkbox/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';

import {
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/dialog/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';

import {
applicationBackgroundColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/drawer/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
applicationBackgroundColor,
bodyFont,
Expand Down
3 changes: 1 addition & 2 deletions packages/nimble-components/src/icon-base/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,8 +28,7 @@ export const registerIcon = (baseName: string, iconClass: IconClass): void => {
const composedIcon = iconClass.compose({
baseName,
template,
styles,
baseClass: iconClass
styles
});

DesignSystem.getOrCreate().withPrefix('nimble').register(composedIcon());
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/icon-base/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
iconSize,
warningColor,
Expand Down
6 changes: 6 additions & 0 deletions packages/nimble-components/src/label-provider/base/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { css } from '@microsoft/fast-element';
import { display } from '../../utilities/style/display';

export const styles = css`
${display('none')}
`;
4 changes: 3 additions & 1 deletion packages/nimble-components/src/label-provider/core/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
filterSearchLabel,
filterNoResultsLabel
} from './label-tokens';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -63,7 +64,8 @@ export class LabelProviderCore
}

const nimbleLabelProviderCore = LabelProviderCore.compose({
baseName: 'label-provider-core'
baseName: 'label-provider-core',
styles
});

DesignSystem.getOrCreate()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import {
richTextToggleBulletedListLabel,
richTextToggleNumberedListLabel
} from './label-tokens';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -43,7 +44,8 @@ export class LabelProviderRichText
}

const nimbleLabelProviderRichText = LabelProviderRichText.compose({
baseName: 'label-provider-rich-text'
baseName: 'label-provider-rich-text',
styles
});

DesignSystem.getOrCreate()
Expand Down
4 changes: 3 additions & 1 deletion packages/nimble-components/src/label-provider/table/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import {
tableGroupRowPlaceholderEmptyLabel,
tableGroupRowPlaceholderNoValueLabel
} from './label-tokens';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -103,7 +104,8 @@ export class LabelProviderTable
}

const nimbleLabelProviderTable = LabelProviderTable.compose({
baseName: 'label-provider-table'
baseName: 'label-provider-table',
styles
});

DesignSystem.getOrCreate()
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/list-option/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';

import {
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/listbox/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';

import {
applicationBackgroundColor,
Expand Down
6 changes: 6 additions & 0 deletions packages/nimble-components/src/mapping/base/styles.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
import { css } from '@microsoft/fast-element';
import { display } from '../../utilities/style/display';

export const styles = css`
${display('none')}
`;
4 changes: 3 additions & 1 deletion packages/nimble-components/src/mapping/empty/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { attr } from '@microsoft/fast-element';
import { Mapping } from '../base';
import { template } from '../base/template';
import type { MappingKey } from '../base/types';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -22,7 +23,8 @@ export class MappingEmpty extends Mapping<MappingKey> {

const emptyMapping = MappingEmpty.compose({
baseName: 'mapping-empty',
template
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(emptyMapping());
export const mappingEmptyTag = 'nimble-mapping-empty';
4 changes: 3 additions & 1 deletion packages/nimble-components/src/mapping/icon/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { template } from '../base/template';
import type { IconSeverity } from '../../icon-base/types';
import { Icon } from '../../icon-base';
import type { MappingKey } from '../base/types';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand Down Expand Up @@ -83,7 +84,8 @@ export class MappingIcon extends Mapping<MappingKey> {

const iconMapping = MappingIcon.compose({
baseName: 'mapping-icon',
template
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(iconMapping());
export const mappingIconTag = 'nimble-mapping-icon';
4 changes: 3 additions & 1 deletion packages/nimble-components/src/mapping/spinner/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { attr } from '@microsoft/fast-element';
import { Mapping } from '../base';
import { template } from '../base/template';
import type { MappingKey } from '../base/types';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -25,7 +26,8 @@ export class MappingSpinner extends Mapping<MappingKey> {

const spinnerMapping = MappingSpinner.compose({
baseName: 'mapping-spinner',
template
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(spinnerMapping());
export const mappingSpinnerTag = 'nimble-mapping-spinner';
4 changes: 3 additions & 1 deletion packages/nimble-components/src/mapping/text/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { attr } from '@microsoft/fast-element';
import { Mapping } from '../base';
import { template } from '../base/template';
import type { MappingKey } from '../base/types';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -22,7 +23,8 @@ export class MappingText extends Mapping<MappingKey> {

const textMapping = MappingText.compose({
baseName: 'mapping-text',
template
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(textMapping());
export const mappingTextTag = 'nimble-mapping-text';
4 changes: 3 additions & 1 deletion packages/nimble-components/src/mapping/user/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { DesignSystem } from '@microsoft/fast-foundation';
import { Mapping } from '../base';
import type { MappingUserKey } from '../base/types';
import { template } from '../base/template';
import { styles } from '../base/styles';

declare global {
interface HTMLElementTagNameMap {
Expand All @@ -19,7 +20,8 @@ export class MappingUser extends Mapping<MappingUserKey> {
}
const mappingUser = MappingUser.compose({
baseName: 'mapping-user',
template
template,
styles
});
DesignSystem.getOrCreate().withPrefix('nimble').register(mappingUser());
export const mappingUserTag = 'nimble-mapping-user';
2 changes: 1 addition & 1 deletion packages/nimble-components/src/menu-button/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { controlHeight, smallPadding } from '../theme-provider/design-tokens';

export const styles = css`
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/menu-item/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import { focusVisible } from '../utilities/style/focus';

import {
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
@@ -1,6 +1,6 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { White } from '@ni/nimble-tokens/dist/styledictionary/js/tokens';
import { display } from '../utilities/style/display';

import {
applicationBackgroundColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/number-field/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../utilities/style/display';
import {
borderRgbPartialColor,
borderHoverColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/patterns/button/styles.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { display } from '../../utilities/style/display';
import { focusVisible } from '../../utilities/style/focus';
import {
actionRgbPartialColor,
Expand Down
2 changes: 1 addition & 1 deletion packages/nimble-components/src/patterns/dropdown/styles.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { css } from '@microsoft/fast-element';
import { display } from '@microsoft/fast-foundation';
import { White } from '@ni/nimble-tokens/dist/styledictionary/js/tokens';
import { display } from '../../utilities/style/display';
import {
applicationBackgroundColor,
bodyFont,
Expand Down
Loading

0 comments on commit 17ddea5

Please sign in to comment.