Skip to content

Commit

Permalink
Merge branch 'master' into layout-components-rc
Browse files Browse the repository at this point in the history
  • Loading branch information
michaeltaranto committed Sep 16, 2024
2 parents 68e2892 + e71ce0c commit 1597378
Show file tree
Hide file tree
Showing 21 changed files with 402 additions and 136 deletions.
95 changes: 95 additions & 0 deletions docs/Removing dividers support from layout components.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
# Removing `dividers` support from layout components

Following the update to SEEK’s Browser Support Policy, we are now able to leverage [CSS gap] in Braid, and more importantly, our layout components. This will be landing in Braid in the v33 release.

Previously, to enable gap-like behaviour layout components iterated over their children, wrapping them in container elements and applying padding. The trade off of this technique was an increased number of DOM elements and the introduction of unwanted space if the child element was hidden or returned `null` — requiring developers to hoist logic to the parent component.

Migrating to CSS gap mitigates all of the above trade offs.

However, because `Stack` and `Tiles` no longer iterate over their children, implementing the `dividers` feature centrally is no longer feasible.

While we could have conditionally maintained this behaviour, it would have resulted in inconsistent edge cases when using `dividers`. E.g. if a child component rendered nothing or a hidden element, the `divider` would still be rendered, resulting in an inconsistent layout.

## Migration guide

Braid already provides the [`Divider` component], so migrating away from the `dividers` prop is a matter of inserting a `Divider` as required into your layout.
How you migrate will depend on whether the children of your layout component are static or being iterated over.

### `Stack`

For `Stack`s with static children you can manually interleave `Divider` components:

```diff
-<Stack space="..." dividers>
+<Stack space="...">
<Component>{item}</Component>
+ <Divider />
<Component>{item}</Component>****
+ <Divider />
<Component>{item}</Component>
</Stack>
```

For `Stack`s with iterable children you can conditionally render `Divider` components, before each item (except the first):

```diff
-<Stack space="..." dividers>
+<Stack space="...">
{items.map((item, index) => (
- <Component>{item}</Component>
+ <Fragment key={...}>
+ {index > 0 ? <Divider /> : null}
+ <Component>{item}</Component>
+ </Fragment>
))}
</Stack>
```

### `Tiles`

For `Tiles` the `dividers` prop was only applied when rendered as a single column.

For this you can conditionally render the `Divider` in a `Stack` with the same spacing as specified on the `Tiles` instance, and hide it on breakpoints showing more than one column.

Here is an example of this with static children:

```diff
-<Tiles space="..." columns={{mobile: 1, tablet: 2}} dividers>
+<Tiles space="..." columns={{mobile: 1, tablet: 2}}>
<Component>{item}</Component>
+ <Stack space="...">
+ <Hidden above="mobile">
+ <Divider />
+ </Hidden>
<Component>{item}</Component>
+ </Stack>
+ <Stack space="...">
+ <Hidden above="mobile">
+ <Divider />
+ </Hidden>
<Component>{item}</Component>
+ </Stack>
</Tiles>
```

Here is an example of this with iterable children:

```diff
-<Tiles space="..." columns={{mobile: 1, tablet: 2}} dividers>
+<Tiles space="..." columns={{mobile: 1, tablet: 2}}>
{items.map((item, index) => (
- <Component>{item}</Component>
+ <Stack space="..." key={...}>
+ {index > 0 ? (
+ <Hidden above="mobile">
+ <Divider />
+ </Hidden>
+ ) : null}
<Component>{item}</Component>
+ </Stack>
))}
</Tiles>
```

[CSS gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/gap
[`Divider` component]: https://seek-oss.github.io/braid-design-system/components/Divider/
3 changes: 3 additions & 0 deletions jest/setupTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,9 @@ import { format, TextEncoder, TextDecoder } from 'util';
global.TextEncoder = TextEncoder;
global.TextDecoder = TextDecoder;

// Mocking for `jsdom`
window.HTMLElement.prototype.scrollIntoView = function () {};

const error = global.console.error;

globalThis.IS_REACT_ACT_ENVIRONMENT = true;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@
"volta": {
"node": "20.17.0"
},
"packageManager": "pnpm@9.9.0",
"packageManager": "pnpm@9.10.0",
"pnpm": {
"patchedDependencies": {
"[email protected]": "patches/[email protected]",
Expand Down
30 changes: 30 additions & 0 deletions packages/braid-design-system/CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,35 @@
# braid-design-system

## 32.24.0

### Minor Changes

- **Autosuggest**: Optimise automatic scrolling to selected suggestion by using native browser methods. ([#1571](https://github.com/seek-oss/braid-design-system/pull/1571))

### Patch Changes

- **Stack, Tiles:** Deprecate `dividers` prop ([#1574](https://github.com/seek-oss/braid-design-system/pull/1574))

In preparation for migrating Braid layout components to use [CSS gap], the `dividers` prop has been deprecated on `Stack` and `Tiles`.

Consumers are encouraged to migrate now in advance of its removal in v33.

#### Migration Guide

See [the migration guide] for details on how to migrate off the `dividers` prop.

[CSS gap]: https://developer.mozilla.org/en-US/docs/Web/CSS/gap
[migration guide]: https://github.com/seek-oss/braid-design-system/blob/master/docs/Removing%20dividers%20support%20from%20layout%20components.md

- **Autosuggest**: Improve handling of `suggestionHighlight` prop when set to `remaining` ([#1572](https://github.com/seek-oss/braid-design-system/pull/1572))

Fixes a bug in `Autosuggest` when using `suggestionHighlight` prop set to `remaining`.
If the input contained multiple words, the highlighted portion would be appended to the end of matching suggestions.

- **Divider:** Ensure full width in flex container ([#1574](https://github.com/seek-oss/braid-design-system/pull/1574))

Ensures the `Divider` component remains full width when used as a flex child inside a flex container.

## 32.23.1

### Patch Changes
Expand Down
4 changes: 2 additions & 2 deletions packages/braid-design-system/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "braid-design-system",
"version": "32.23.1",
"version": "32.24.0",
"description": "Themeable design system for the SEEK Group",
"homepage": "https://seek-oss.github.io/braid-design-system/",
"bugs": {
Expand Down Expand Up @@ -247,7 +247,7 @@
"react-dom": "^18.3.1",
"react-router-dom": "^6.3.0",
"sanitize-html": "^2.12.1",
"sku": "13.1.1",
"sku": "13.1.2",
"storybook": "7.6.20",
"svgo": "^2.8.0",
"title-case": "^3.0.3"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@ import {
Column,
Columns,
TextField,
Inline,
} from '../';
import { IconHelp, IconLanguage } from '../icons';
import { highlightSuggestions } from './Autosuggest';
Expand Down Expand Up @@ -478,7 +477,7 @@ const docs: ComponentDocs = {
source(
<>
{setDefaultState('textfield', 'App')}
{setDefaultState('suggestion', 'Apples')}
{setDefaultState('suggestion', 'Apples and Bananas')}

<Stack space="large">
<TextField
Expand All @@ -494,7 +493,7 @@ const docs: ComponentDocs = {
<Text size="small" tone="secondary">
Highlight <Strong>{highlightType}</Strong>
</Text>
<Inline space="none">
<Text>
{parseHighlights(
getState('suggestion'),
highlightSuggestions(
Expand All @@ -504,15 +503,14 @@ const docs: ComponentDocs = {
? 'matching'
: 'remaining',
).map(({ start, end }) => [start, end]),
).map((part, index) => (
<Text
key={index}
weight={part.highlight ? 'strong' : 'regular'}
>
{part.text}
</Text>
))}
</Inline>
).map((part, index) =>
part.highlight ? (
<Strong key={index}>{part.text}</Strong>
) : (
part.text
),
)}
</Text>
</Stack>
</Column>
))}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ import { getNextIndex } from '../private/getNextIndex';
import { normalizeKey } from '../private/normalizeKey';
import { ClearField } from '../private/Field/ClearField';
import { smoothScroll } from '../private/smoothScroll';
import { useScrollIntoView } from './useScrollIntoView';
import { useResponsiveValue } from '../useResponsiveValue/useResponsiveValue';
import { RemoveScroll } from 'react-remove-scroll';
import {
Expand All @@ -44,6 +43,7 @@ import {
type AutosuggestTranslations,
autosuggest,
} from '../../translations/en';
import { reverseMatches } from './reverseMatches';

import * as styles from './Autosuggest.css';

Expand Down Expand Up @@ -346,14 +346,13 @@ export function highlightSuggestions(
value: string,
variant: HighlightOptions = 'matching',
): SuggestionMatch {
const matches = matchHighlights(suggestion, value);
const matchedHighlights = matchHighlights(suggestion, value);
const matches =
variant === 'matching'
? matchedHighlights
: reverseMatches(suggestion, matchedHighlights);

const formattedMatches =
variant === 'remaining'
? matches.map(([_, end]) => ({ start: end, end: suggestion.length }))
: matches.map(([start, end]) => ({ start, end }));

return formattedMatches;
return matches.map(([start, end]) => ({ start, end }));
}

export const Autosuggest = forwardRef(function <Value>(
Expand Down Expand Up @@ -582,7 +581,7 @@ export const Autosuggest = forwardRef(function <Value>(
? document.getElementById(getItemId(id, highlightedIndex))
: null;

useScrollIntoView(highlightedItem, menuRef.current);
highlightedItem?.scrollIntoView({ block: 'nearest' });

useEffect(() => {
dispatch({
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
import { type Match, reverseMatches } from './reverseMatches';

describe('reverseMatches', () => {
it('should return intervals for non-matching parts of the suggestion', () => {
const suggestion = 'Apples etc';
const matches: Match[] = [
[2, 4],
[6, 8],
];
const expected: Match[] = [
[0, 2],
[4, 6],
[8, 10],
];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});

it('should handle no matches', () => {
const suggestion = 'Apple';
const matches: Match[] = [];
const expected: Match[] = [[0, 5]];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});

it('should handle matches that cover the entire suggestion', () => {
const suggestion = 'Apple';
const matches: Match[] = [[0, 5]];
const expected: Match[] = [];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});

it('should handle matches at the start of the suggestion', () => {
const suggestion = 'Apple';
const matches: Match[] = [[0, 2]];
const expected: Match[] = [[2, 5]];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});

it('should handle matches at the end of the suggestion', () => {
const suggestion = 'Apple';
const matches: Match[] = [[3, 5]];
const expected: Match[] = [[0, 3]];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});

it('should handle matches for a single character suggestion', () => {
const suggestion = 'A';
const matches: Match[] = [[0, 1]];
const expected: Match[] = [];

expect(reverseMatches(suggestion, matches)).toEqual(expected);
});
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
/*
In each Match,
- First number: index of the first highlighted character in the match
- Second number: index of the last highlighted character in the match *plus one*
This matches the format expected by the parse function
See https://github.com/moroshko/autosuggest-highlight?tab=readme-ov-file#parsetext-matches
*/

export type Match = [number, number];

export function reverseMatches(suggestion: string, matches: Match[]): Match[] {
const suggestionLength = suggestion.length;
const reversedMatches: Match[] = [];

let currentStart = 0;

for (const [start, end] of matches) {
if (currentStart < start) {
reversedMatches.push([currentStart, start]);
}

currentStart = end;
}

if (currentStart < suggestionLength) {
reversedMatches.push([currentStart, suggestionLength]);
}

return reversedMatches;
}

This file was deleted.

Loading

0 comments on commit 1597378

Please sign in to comment.