Skip to content

Commit

Permalink
fix(segment): improves loading of the component and solves nested ele…
Browse files Browse the repository at this point in the history
…mnts sizing issue (#2358)

* fix(segment): improves loading of the component and solves nested size issues
  • Loading branch information
amir-ba authored Oct 21, 2024
1 parent e0ff157 commit 299be7d
Show file tree
Hide file tree
Showing 6 changed files with 151 additions and 65 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ exports[`Segment should match standard snapshot 1`] = `
</div>
</button>
</mock:shadow-root>
Label
<span>
Label
</span>
</scale-segment>
`;
36 changes: 18 additions & 18 deletions packages/components/src/components/segment/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@

## Properties

| Property | Attribute | Description | Type | Default |
| ---------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------- | -------------- |
| `adjacentSiblings` | `adjacent-siblings` | | `"left" \| "leftright" \| "right"` | `undefined` |
| `ariaDescriptionTranslation` | `aria-description-translation` | a11y text for getting meaningful value. `$buttonNumber` and `$selected` are template variables and will be replaces by their corresponding properties. | `string` | `'$selected'` |
| `ariaLabelSegment` | `aria-label-segment` | (optional) aria-label attribute needed for icon-only segments | `string` | `undefined` |
| `ariaLangDeselected` | `aria-lang-deselected` | (optional) translation of 'deselected | `string` | `'deselected'` |
| `ariaLangSelected` | `aria-lang-selected` | (optional) translation of 'selected | `string` | `'selected'` |
| `disabled` | `disabled` | (optional) If `true`, the segment is disabled | `boolean` | `false` |
| `hasIcon` | `has-icon` | (optional) position within group | `boolean` | `undefined` |
| `iconOnly` | `icon-only` | (optional) position within group | `boolean` | `undefined` |
| `position` | `position` | (optional) position within group | `number` | `undefined` |
| `segmentId` | `segment-id` | (optional) segment's id | `string` | `undefined` |
| `selected` | `selected` | (optional) If `true`, the segment is selected | `boolean` | `false` |
| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `string` | `undefined` |
| `size` | `size` | (optional) The size of the segment | `"large" \| "medium" \| "small"` | `'small'` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| `textOnly` | `text-only` | (optional) position within group | `boolean` | `undefined` |
| `width` | `width` | (optional) Segment width set to ensure that all segments have the same width | `string` | `undefined` |
| Property | Attribute | Description | Type | Default |
| ---------------------------- | ------------------------------ | ------------------------------------------------------------------------------------------------------------------------------------------------------ | ---------------------------------- | ------------------ |
| `adjacentSiblings` | `adjacent-siblings` | | `"left" \| "leftright" \| "right"` | `undefined` |
| `ariaDescriptionTranslation` | `aria-description-translation` | a11y text for getting meaningful value. `$buttonNumber` and `$selected` are template variables and will be replaces by their corresponding properties. | `string` | `'$selected'` |
| `ariaLabelSegment` | `aria-label-segment` | (optional) aria-label attribute needed for icon-only segments | `string` | `undefined` |
| `ariaLangDeselected` | `aria-lang-deselected` | (optional) translation of 'deselected | `string` | `'deselected'` |
| `ariaLangSelected` | `aria-lang-selected` | (optional) translation of 'selected | `string` | `'selected'` |
| `disabled` | `disabled` | (optional) If `true`, the segment is disabled | `boolean` | `false` |
| `hasIcon` | `has-icon` | (optional) position within group | `boolean` | `undefined` |
| `iconOnly` | `icon-only` | (optional) position within group | `boolean` | `undefined` |
| `position` | `position` | (optional) position within group | `number` | `undefined` |
| `segmentId` | `segment-id` | (optional) segment's id | `string` | `'segment-' + i++` |
| `selected` | `selected` | (optional) If `true`, the segment is selected | `boolean` | `false` |
| `selectedIndex` | `selected-index` | (optional) the index of the currently selected segment in the segmented-button | `string` | `undefined` |
| `size` | `size` | (optional) The size of the segment | `"large" \| "medium" \| "small"` | `'small'` |
| `styles` | `styles` | (optional) Injected CSS styles | `string` | `undefined` |
| `textOnly` | `text-only` | (optional) position within group | `boolean` | `undefined` |
| `width` | `width` | (optional) Segment width set to ensure that all segments have the same width | `string` | `undefined` |


## Events
Expand Down
9 changes: 3 additions & 6 deletions packages/components/src/components/segment/segment.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ export class Segment {
/** (optional) If `true`, the segment is disabled */
@Prop() disabled?: boolean = false;
/** (optional) segment's id */
@Prop({ reflect: true, mutable: true }) segmentId?: string;
@Prop({ reflect: true }) segmentId?: string = 'segment-' + i++;
/** (optional) aria-label attribute needed for icon-only segments */
@Prop() ariaLabelSegment: string;
/** (optional) Segment width set to ensure that all segments have the same width */
Expand Down Expand Up @@ -84,13 +84,10 @@ export class Segment {
async setFocus() {
this.focusableElement.focus();
}

componentWillLoad() {
if (this.segmentId == null) {
this.segmentId = 'segment-' + i++;
}
this.handleIcon();
}
componentDidUpdate() {
componentWillUpdate() {
this.handleIcon();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`SegmentedButton should match selected button snapshot 1`] = `
<scale-segmented-button>
<mock:shadow-root>
<div aria-label="segment button with 2" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(2, 0px);">
<div aria-label="segment button with 2" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(2, auto);">
<slot></slot>
</div>
</mock:shadow-root>
Expand All @@ -19,7 +19,7 @@ exports[`SegmentedButton should match selected button snapshot 1`] = `
exports[`SegmentedButton should match standard snapshot 1`] = `
<scale-segmented-button>
<mock:shadow-root>
<div aria-label="segment button with 4" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(4, 14px);">
<div aria-label="segment button with 4" class="segmented-button segmented-button--small" part="segmented-button small" role="group" style="grid-template-columns: repeat(4, auto);">
<slot></slot>
</div>
</mock:shadow-root>
Expand All @@ -36,4 +36,4 @@ exports[`SegmentedButton should match standard snapshot 1`] = `
Label
</scale-segment>
</scale-segmented-button>
`;
`;
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export class SegmentedButton {
/** (optional) Allow more than one button to be selected */
@Prop() multiSelect: boolean = false;
/** (optional) the index of the selected segment */
@Prop() selectedIndex?: number;
@Prop({ mutable: true }) selectedIndex?: number;
/** (optional) If `true`, the button is disabled */
@Prop({ reflect: true }) disabled?: boolean = false;
/** (optional) If `true`, expand to container width */
Expand Down Expand Up @@ -119,47 +119,52 @@ export class SegmentedButton {
});
}

componentDidLoad() {
componentWillLoad() {
const tempState: SegmentStatus[] = [];
const segments = this.getAllSegments();
this.slottedSegments = segments.length;
const longestButtonWidth = this.getLongestButtonWidth();
segments.forEach((segment) => {
this.position++;
segments.forEach((segment, i) => {
tempState.push({
id: segment.getAttribute('segment-id') || segment.segmentId,
selected: segment.hasAttribute('selected') || segment.selected,
});
segment.setAttribute('position', this.position.toString());
segment.setAttribute('position', `${i + 1}`);
segment.setAttribute(
'aria-description-translation',
'$position $selected'
);
});
this.setState(tempState);
this.selectedIndex = this.getSelectedIndex();
this.showHelperText = this.shouldShowHelperText();
}
componentDidLoad() {
const longestButtonWidth = this.getLongestButtonWidth();
if (!this.fullWidth) {
this.container.style.gridTemplateColumns = `repeat(${
this.hostElement.children.length
}, ${Math.ceil(longestButtonWidth)}px)`;
this.container.style.gridTemplateColumns = longestButtonWidth
? `repeat(${this.hostElement.children.length}, ${Math.ceil(
longestButtonWidth
)}px)`
: `repeat(${this.hostElement.children.length}, auto)`;
} else {
this.container.style.display = 'flex';
}

this.selectedIndex = this.getSelectedIndex();
this.propagatePropsToChildren();
this.position = 0;
this.status = tempState;
this.setState(tempState);
}

componentWillUpdate() {
this.selectedIndex = this.getSelectedIndex();
this.showHelperText = false;
this.showHelperText = this.shouldShowHelperText();
}
shouldShowHelperText() {
let showHelperText = false;
if (
this.invalid &&
this.status.filter((e) => e.selected === true).length <= 0
) {
this.showHelperText = true;
showHelperText = true;
}
return showHelperText;
}

getSelectedIndex() {
Expand Down Expand Up @@ -195,27 +200,29 @@ export class SegmentedButton {
// all segmented buttons should have the same width, based on the largest one
getLongestButtonWidth() {
let tempWidth = 0;
Array.from(this.hostElement.children).forEach((child) => {
const selected = child.hasAttribute('selected');
const iconOnly = child.hasAttribute('icon-only');
const checkmark =
this.size === 'small'
? CHECKMARK_WIDTH_SMALL
: this.size === 'medium'
? CHECKMARK_WIDTH_MEDIUM
: CHECKMARK_WIDTH_LARGE;
if (selected || iconOnly) {
tempWidth =
child.getBoundingClientRect().width > tempWidth
? child.getBoundingClientRect().width
: tempWidth;
} else {
tempWidth =
child.getBoundingClientRect().width + checkmark > tempWidth
? child.getBoundingClientRect().width + checkmark
: tempWidth;
}
});
Array.from(this.hostElement.children)
.filter((child) => child.getBoundingClientRect().width)
.forEach((child) => {
const selected = child.hasAttribute('selected');
const iconOnly = child.hasAttribute('icon-only');
const checkmark =
this.size === 'small'
? CHECKMARK_WIDTH_SMALL
: this.size === 'medium'
? CHECKMARK_WIDTH_MEDIUM
: CHECKMARK_WIDTH_LARGE;
if (selected || iconOnly) {
tempWidth =
child.getBoundingClientRect().width > tempWidth
? child.getBoundingClientRect().width
: tempWidth;
} else {
tempWidth =
child.getBoundingClientRect().width + checkmark > tempWidth
? child.getBoundingClientRect().width + checkmark
: tempWidth;
}
});
return tempWidth;
}

Expand Down
80 changes: 80 additions & 0 deletions packages/components/src/html/segment-button.html
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
<!DOCTYPE html>
<html dir="ltr" lang="en">
<head>
<meta charset="utf-8" />
<meta
name="viewport"
content="width=device-width, initial-scale=1.0, minimum-scale=1.0, maximum-scale=5.0"
/>
<title>Segment Buttons</title>

<script type="module" src="build/scale-components.esm.js"></script>
<script nomodule src="build/scale-components.js"></script>
<link rel="stylesheet" href="build/scale-components.css" />
<style>
section {
padding: 1rem;
}

.bg-black {
background: black;
color: white;
}
</style>
</head>

<body>
<scale-collapsible>
<div slot="heading">Open Me for Disaster</div>
<scale-segmented-button>
<scale-segment selected>Apple</scale-segment>
<scale-segment>One+</scale-segment>
<scale-segment>Samsung</scale-segment>
<scale-segment>Huawei</scale-segment>
</scale-segmented-button>
</scale-collapsible>
<div style="height: 20px"></div>
<scale-segmented-button>
<scale-segment>Apple</scale-segment>
<scale-segment>One+</scale-segment>
<scale-segment>Samsung</scale-segment>
<scale-segment>Huawei</scale-segment>
</scale-segmented-button>
<div style="height: 20px"></div>
<scale-segmented-button>
<scale-segment selected>Label</scale-segment>
<scale-segment selected>Label</scale-segment>
</scale-segmented-button>
<div style="height: 20px"></div>
<scale-segmented-button invalid>
<scale-segment>
<scale-icon-device-device-phone slot="segment-icon">
</scale-icon-device-device-phone>
Apple
</scale-segment>
<scale-segment>
<scale-icon-device-device-phone slot="segment-icon">
</scale-icon-device-device-phone>
One+
</scale-segment>
<scale-segment>
<scale-icon-device-device-phone slot="segment-icon">
</scale-icon-device-device-phone>
Samsung
</scale-segment>
<scale-segment>
<scale-icon-device-device-phone slot="segment-icon">
</scale-icon-device-device-phone>
Huawei
</scale-segment>
</scale-segmented-button>
<div style="height: 20px"></div>

<scale-segmented-button multi-select>
<scale-segment selected>Apple</scale-segment>
<scale-segment selected>One+</scale-segment>
<scale-segment>Samsung</scale-segment>
<scale-segment>Huawei</scale-segment>
</scale-segmented-button>
</body>
</html>

0 comments on commit 299be7d

Please sign in to comment.