feat(ui5-segmentedbutton): initial implementation #1164
Conversation
packages/main/src/SegmentedButton.js
Outdated
| this.widths = this.buttons.map(button => button.offsetWidth); | ||
| } | ||
|
|
||
| syncSelection() { |
There was a problem hiding this comment.
This is not really synchronization. It's deselecting, if there are many selected. Think of a better name.
packages/main/src/SegmentedButton.js
Outdated
| * @param {HTMLElement} pressedButton the pressed button. | ||
| * @public | ||
| */ | ||
| pressChange: { |
There was a problem hiding this comment.
Was this name agreed upon? Press change sounds ok for the toggle button, but for segmented button it's more like "selectionChange". How is it in OpenUI5?
packages/main/src/SegmentedButton.js
Outdated
| return this.toggle(event); | ||
| } | ||
|
|
||
| toggle(event) { |
There was a problem hiding this comment.
This function seems to be only called from onclick. Just move the code there
packages/main/src/SegmentedButton.js
Outdated
| } | ||
|
|
||
| toggle(event) { | ||
| if (event.target !== this.pressedButton) { |
There was a problem hiding this comment.
This is confusing a bit. It's the same as ._pressedButton. Do we need "pressedButton"? Is this the public API? Even if so, just check for ._pressedButton here
packages/main/src/SegmentedButton.js
Outdated
| this.buttons.forEach(button => { | ||
| button.pressed = false; | ||
| }); | ||
| this._pressedButton.pressed = true; |
There was a problem hiding this comment.
What does this line do? If we set pressed to false to all buttons in the loop, isn't this unnecessary?
There was a problem hiding this comment.
We need this in order to leave the last pressed button, after we deselect all buttons.
packages/main/src/SegmentedButton.js
Outdated
| pressedButton: this._pressedButton, | ||
| }); | ||
| } | ||
| event.target.pressed = true; |
There was a problem hiding this comment.
Better use ._pressedButton here.
| class="ui5-segmentedbutton-root" | ||
| > | ||
| {{#each buttons}} | ||
| <slot name="{{this._individualSlot}}"></slot> |
There was a problem hiding this comment.
I am wondering if we need the individual slot assignment as we do not wrap the buttons in spans or anything else?
<div
@click="{{_onclick}}"
class="ui5-segmentedbutton-root"
>
<slot></slot>
</div>…ashkov/ui5-webcomponents into implement-segmented-button
packages/main/src/SegmentedButton.js
Outdated
| } | ||
|
|
||
| _handleResize() { | ||
| const documentWidth = document.body.clientWidth; |
There was a problem hiding this comment.
I found one issue:
- First, open the test page in large size
- Then, shrink the page to the point the component gets width: 100%;
- Then, expand the page back - the segmented button remains with width: 100%,
but should not.
packages/main/src/SegmentedButton.js
Outdated
| * | ||
| * The <code>SegmentedButton</code> shows a group of buttons. When the user clicks or taps | ||
| * one of the buttons, it stays in a pressed state. It automatically resizes the buttons | ||
| * to fit proportionally within the control. When no width is set, the control uses the available width. |
There was a problem hiding this comment.
use "component", instead of "control"
Initial implementation of the ui5-segmentedbutton component.
Fixes: #1180