Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 6 additions & 5 deletions cypress/component/Tooltip.cy.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -245,11 +245,12 @@ describe('<Tooltip/>', () => {

cy.get(tooltip).should('not.be.visible')

cy.get('[data-testid="trigger"]')
.realHover()
.then(() => {
cy.get(tooltip).should('be.visible')
})
cy.get('[data-testid="trigger"]').realHover()

// Verify tooltip is rendered and accessible (avoid Cypress's "covered by" check)
cy.get(tooltip).should('exist')
cy.get(tooltip).should('have.css', 'display', 'block')
cy.contains('Hello. I\'m a tool tip').should('exist')

cy.get(tooltip)
.realPress('Escape')
Expand Down
2 changes: 1 addition & 1 deletion docs/contributor-docs/migrating-to-new-tokens.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ Changes needed:

If tokens are from a different (usually parent) components, add the `componentID` of that component as second paramater of `@withStyle` and use that name in the `generateStyle` function in `style.ts`: `NewComponentTypes['ParentComponentNameWithTheTokens']`

`generateStyle` accepts a third parameter as well, which are the `sharedTokens`. These provide tokens for shared behaviors such as focus rings, shadows or margins. `'@instructure/emotion'` has various util functions that uses these, such as `calcMarginFromShorthand` and `calcFocusOutlineStyles`.
`generateStyle` accepts a third parameter as well, which are the `sharedTokens`. These provide tokens for shared behaviors such as focus rings, shadows or margins. `'@instructure/emotion'` has various util functions that uses these, such as `calcSpacingFromShorthand` and `calcFocusOutlineStyles`.

## Removing View

Expand Down
17 changes: 16 additions & 1 deletion docs/guides/upgrade-guide.md
Original file line number Diff line number Diff line change
Expand Up @@ -104,9 +104,10 @@ type: example
<TextInput renderLabel="Name" placeholder="Doe, John Doe"/>
</InstUISettingsProvider>
```

### Breadcrumb

#### New tokens
#### New tokens

- gapSm - Gap spacing for small size breadcrumbs
- gapMd - Gap spacing for medium size breadcrumbs
Expand Down Expand Up @@ -250,6 +251,20 @@ type: example
- theme variable `borderStyle` is now removed
- theme variable `position` is now removed

### View

#### Theme variable changes

| Old Variable | Status | Notes |
| ---------------------------------------------------------------------------------------------------------------------------------------- | ------- | ------------------------------------------- |
| `arrowSize` | Removed | Moved to ContextView component |
| `marginXxxSmall`, `marginXxSmall`, `marginXSmall`, `marginSmall`, `marginMedium`, `marginLarge`, `marginXLarge`, `marginXxLarge` | Removed | Use `sharedTokens.spacing` |
| `paddingXxxSmall`, `paddingXxSmall`, `paddingXSmall`, `paddingSmall`, `paddingMedium`, `paddingLarge`, `paddingXLarge`, `paddingXxLarge` | Removed | Use `sharedTokens.spacing` |
| `shadowDepth1`, `shadowDepth2`, `shadowDepth3` | Removed | Use `sharedTokens.boxShadow.elevation1/2/3` |
| `shadowResting`, `shadowAbove`, `shadowTopmost` | Removed | Use `sharedTokens.boxShadow.elevation*` |
| `borderRadiusSmall`, `borderRadiusMedium`, `borderRadiusLarge` | Removed | Use `sharedTokens.radius*` |
| `borderWidthSmall`, `borderWidthMedium`, `borderWidthLarge` | Removed | Use `sharedTokens.width*` |

## Codemods

To ease the upgrade, we provide codemods that will automate most of the changes. Pay close attention to its output, it cannot refactor complex code! The codemod scripts can be run via the following commands:
Expand Down
2 changes: 1 addition & 1 deletion packages/emotion/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export {
getShorthandPropValue,
mirrorShorthandCorners,
mirrorShorthandEdges,
calcMarginFromShorthand,
calcSpacingFromShorthand,
calcFocusOutlineStyles
} from './styleUtils'

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,9 +23,9 @@
*/

import { describe, it, expect, vi } from 'vitest'
import { calcMarginFromShorthand } from '../calcMarginFromShorthand'
import { calcSpacingFromShorthand } from '../calcSpacingFromShorthand'

describe('calcMarginFromShorthand', () => {
describe('calcSpacingFromShorthand', () => {
const spacingMap = {
space0: '0px',
space4: '4px',
Expand All @@ -47,73 +47,73 @@ describe('calcMarginFromShorthand', () => {

describe('single token values', () => {
it('should resolve a direct key to its value', () => {
expect(calcMarginFromShorthand('space4', spacingMap)).toBe('4px')
expect(calcSpacingFromShorthand('space4', spacingMap)).toBe('4px')
})

it('should resolve space0 to 0px', () => {
expect(calcMarginFromShorthand('space0', spacingMap)).toBe('0px')
expect(calcSpacingFromShorthand('space0', spacingMap)).toBe('0px')
})

it('should resolve space16 to 16px', () => {
expect(calcMarginFromShorthand('space16', spacingMap)).toBe('16px')
expect(calcSpacingFromShorthand('space16', spacingMap)).toBe('16px')
})
})

describe('multiple token values (CSS shorthand)', () => {
it('should handle two token values', () => {
expect(calcMarginFromShorthand('space4 space8', spacingMap)).toBe('4px 8px')
expect(calcSpacingFromShorthand('space4 space8', spacingMap)).toBe('4px 8px')
})

it('should handle three token values', () => {
expect(calcMarginFromShorthand('space4 gap.sm space16', spacingMap)).toBe('4px 2px 16px')
expect(calcSpacingFromShorthand('space4 gap.sm space16', spacingMap)).toBe('4px 2px 16px')
})

it('should handle four token values', () => {
expect(calcMarginFromShorthand('space0 space4 space8 space16', spacingMap)).toBe('0px 4px 8px 16px')
expect(calcSpacingFromShorthand('space0 space4 space8 space16', spacingMap)).toBe('0px 4px 8px 16px')
})
})

describe('nested token paths with dot notation', () => {
it('should resolve single-level nested path', () => {
expect(calcMarginFromShorthand('gap.sm', spacingMap)).toBe('2px')
expect(calcSpacingFromShorthand('gap.sm', spacingMap)).toBe('2px')
})

it('should resolve two-level nested path', () => {
expect(calcMarginFromShorthand('gap.nested.xl', spacingMap)).toBe('24px')
expect(calcSpacingFromShorthand('gap.nested.xl', spacingMap)).toBe('24px')
})

it('should handle multiple nested paths', () => {
expect(calcMarginFromShorthand('gap.sm gap.nested.xl', spacingMap)).toBe('2px 24px')
expect(calcSpacingFromShorthand('gap.sm gap.nested.xl', spacingMap)).toBe('2px 24px')
})

it('should handle padding.small', () => {
expect(calcMarginFromShorthand('padding.small', spacingMap)).toBe('4px')
expect(calcSpacingFromShorthand('padding.small', spacingMap)).toBe('4px')
})

it('should handle padding.large', () => {
expect(calcMarginFromShorthand('padding.large', spacingMap)).toBe('16px')
expect(calcSpacingFromShorthand('padding.large', spacingMap)).toBe('16px')
})
})

describe('mixing direct keys and nested paths', () => {
it('should handle space0 and gap.sm', () => {
expect(calcMarginFromShorthand('space0 gap.sm', spacingMap)).toBe('0px 2px')
expect(calcSpacingFromShorthand('space0 gap.sm', spacingMap)).toBe('0px 2px')
})

it('should handle padding.small and gap.md', () => {
expect(calcMarginFromShorthand('padding.small gap.md', spacingMap)).toBe('4px 8px')
expect(calcSpacingFromShorthand('padding.small gap.md', spacingMap)).toBe('4px 8px')
})

it('should handle four mixed values', () => {
expect(calcMarginFromShorthand('space4 gap.sm padding.large space16', spacingMap)).toBe('4px 2px 16px 16px')
expect(calcSpacingFromShorthand('space4 gap.sm padding.large space16', spacingMap)).toBe('4px 2px 16px 16px')
})
})

describe('fallback for non-existent tokens', () => {
it('should return the original token when not found in spacingMap', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('nonExistent', spacingMap)
const result = calcSpacingFromShorthand('nonExistent', spacingMap)
expect(result).toBe('nonExistent')
expect(consoleWarnSpy).toHaveBeenCalledWith('Theme token path "nonExistent" not found in theme.')

Expand All @@ -123,7 +123,7 @@ describe('calcMarginFromShorthand', () => {
it('should handle CSS values like auto', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('auto', spacingMap)
const result = calcSpacingFromShorthand('auto', spacingMap)
expect(result).toBe('auto')

consoleWarnSpy.mockRestore()
Expand All @@ -132,7 +132,7 @@ describe('calcMarginFromShorthand', () => {
it('should handle direct CSS values like 10px', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('10px', spacingMap)
const result = calcSpacingFromShorthand('10px', spacingMap)
expect(result).toBe('10px')

consoleWarnSpy.mockRestore()
Expand All @@ -141,7 +141,7 @@ describe('calcMarginFromShorthand', () => {
it('should handle mixed valid tokens and CSS values', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('auto 10px', spacingMap)
const result = calcSpacingFromShorthand('auto 10px', spacingMap)
expect(result).toBe('auto 10px')

consoleWarnSpy.mockRestore()
Expand All @@ -150,7 +150,7 @@ describe('calcMarginFromShorthand', () => {
it('should handle mixed valid tokens and invalid nested paths', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('space4 gap.invalid', spacingMap)
const result = calcSpacingFromShorthand('space4 gap.invalid', spacingMap)
expect(result).toBe('4px gap.invalid')
expect(consoleWarnSpy).toHaveBeenCalledWith('Theme token path "gap.invalid" not found in theme.')

Expand All @@ -160,7 +160,7 @@ describe('calcMarginFromShorthand', () => {
it('should handle deeply nested invalid path', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('gap.nested.xl.invalid', spacingMap)
const result = calcSpacingFromShorthand('gap.nested.xl.invalid', spacingMap)
expect(result).toBe('gap.nested.xl.invalid')
expect(consoleWarnSpy).toHaveBeenCalledWith('Theme token path "gap.nested.xl.invalid" not found in theme.')

Expand All @@ -169,32 +169,32 @@ describe('calcMarginFromShorthand', () => {
})

describe('undefined and edge cases', () => {
it('should return "0" for undefined value', () => {
expect(calcMarginFromShorthand(undefined, spacingMap)).toBe('0')
it('should return undefined for undefined value', () => {
expect(calcSpacingFromShorthand(undefined, spacingMap)).toBe(undefined)
})

it('should handle empty string', () => {
expect(calcMarginFromShorthand('', spacingMap)).toBe('')
expect(calcSpacingFromShorthand('', spacingMap)).toBe(undefined)
})

it('should handle string with only whitespace', () => {
expect(calcMarginFromShorthand(' ', spacingMap)).toBe('')
expect(calcSpacingFromShorthand(' ', spacingMap)).toBe('')
})

it('should handle extra spaces between tokens', () => {
expect(calcMarginFromShorthand('space4 space8', spacingMap)).toBe('4px 8px')
expect(calcSpacingFromShorthand('space4 space8', spacingMap)).toBe('4px 8px')
})

it('should trim leading and trailing whitespace', () => {
expect(calcMarginFromShorthand(' space4 space8 ', spacingMap)).toBe('4px 8px')
expect(calcSpacingFromShorthand(' space4 space8 ', spacingMap)).toBe('4px 8px')
})
})

describe('console warnings', () => {
it('should warn when a direct key is not found', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

calcMarginFromShorthand('invalidToken', spacingMap)
calcSpacingFromShorthand('invalidToken', spacingMap)
expect(consoleWarnSpy).toHaveBeenCalledWith('Theme token path "invalidToken" not found in theme.')

consoleWarnSpy.mockRestore()
Expand All @@ -203,7 +203,7 @@ describe('calcMarginFromShorthand', () => {
it('should warn when a nested path is not found', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

calcMarginFromShorthand('gap.invalid', spacingMap)
calcSpacingFromShorthand('gap.invalid', spacingMap)
expect(consoleWarnSpy).toHaveBeenCalledWith('Theme token path "gap.invalid" not found in theme.')

consoleWarnSpy.mockRestore()
Expand All @@ -212,7 +212,7 @@ describe('calcMarginFromShorthand', () => {
it('should warn for each invalid token in a multi-token value', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

calcMarginFromShorthand('invalid1 invalid2', spacingMap)
calcSpacingFromShorthand('invalid1 invalid2', spacingMap)
expect(consoleWarnSpy).toHaveBeenCalledTimes(2)
expect(consoleWarnSpy).toHaveBeenNthCalledWith(1, 'Theme token path "invalid1" not found in theme.')
expect(consoleWarnSpy).toHaveBeenNthCalledWith(2, 'Theme token path "invalid2" not found in theme.')
Expand All @@ -223,17 +223,17 @@ describe('calcMarginFromShorthand', () => {

describe('complex scenarios', () => {
it('should handle all direct keys', () => {
expect(calcMarginFromShorthand('space0 space4 space8 space16', spacingMap)).toBe('0px 4px 8px 16px')
expect(calcSpacingFromShorthand('space0 space4 space8 space16', spacingMap)).toBe('0px 4px 8px 16px')
})

it('should handle all nested paths', () => {
expect(calcMarginFromShorthand('gap.sm gap.md gap.lg gap.nested.xl', spacingMap)).toBe('2px 8px 16px 24px')
expect(calcSpacingFromShorthand('gap.sm gap.md gap.lg gap.nested.xl', spacingMap)).toBe('2px 8px 16px 24px')
})

it('should handle mix of everything', () => {
const consoleWarnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {})

const result = calcMarginFromShorthand('space4 gap.md auto padding.small', spacingMap)
const result = calcSpacingFromShorthand('space4 gap.md auto padding.small', spacingMap)
expect(result).toBe('4px 8px auto 4px')

consoleWarnSpy.mockRestore()
Expand Down
3 changes: 3 additions & 0 deletions packages/emotion/src/styleUtils/calcFocusOutlineStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -53,12 +53,14 @@ const calcFocusOutlineStyles = (
focusPosition?: 'offset' | 'inset'
shouldAnimateFocus?: boolean
focusWithin?: boolean
withFocusOutline?: boolean
}
) => {
const focusColor = params?.focusColor ?? 'info'
const focusPosition = params?.focusPosition ?? 'offset'
const shouldAnimateFocus = params?.shouldAnimateFocus ?? true
const focusWithin = params?.focusWithin ?? false
const withFocusOutline = params?.withFocusOutline ?? false

const focusColorVariants = {
info: theme.infoColor,
Expand All @@ -80,6 +82,7 @@ const calcFocusOutlineStyles = (
outlineOffset: '-0.8rem',
outlineStyle: 'solid',
outlineColor: alpha(outlineStyle.outlineColor, 0),
...(withFocusOutline && outlineStyle),
'&:focus': {
...outlineStyle,
'&:hover, &:active': {
Expand Down
Loading
Loading