feat(label-value): refactor to use lit#1005
Conversation
* Retained deprecated "dense" prop, if used will set "inline" via willUpdate() * Tests Passing
jasonsubers-tyler
added
the
skip-release
|
|
| * @cssproperty --forge-label-value-align | ||
| * @cssproperty --forge-label-value-label-spacing | ||
| * @cssproperty --forge-label-value-label-block-start-spacing | ||
| * @cssproperty --forge-label-value-label-block-end-spacing | ||
| * @cssproperty --forge-label-value-label-color | ||
| * @cssproperty --forge-label-value-icon-spacing | ||
| * @cssproperty --forge-label-value-inline-label-spacing | ||
| * @cssproperty --forge-label-value-empty-color | ||
| * @cssproperty --forge-label-value-empty-style | ||
| * | ||
| * @cssproperty --forge-label-value-align - Aligns the label and value. Possible values: `start` (default), `center`, `end`. | ||
| * @cssproperty --forge-label-value-label-spacing - The spacing between the label and value. | ||
| * @cssproperty --forge-label-value-label-block-start-spacing - The block start spacing for the label. | ||
| * @cssproperty --forge-label-value-label-block-end-spacing - The block end spacing for the label. | ||
| * @cssproperty --forge-label-value-label-color - The color to apply to the label. | ||
| * @cssproperty --forge-label-value-icon-spacing - The spacing between the icon and the label. | ||
| * @cssproperty --forge-label-value-inline-label-spacing - The spacing between the label and value when displayed inline. | ||
| * @cssproperty --forge-label-value-empty-color - The color to apply to the value when empty. | ||
| * @cssproperty --forge-label-value-empty-style - The font-style to apply to the value when empty. | ||
| * @slot label | ||
| * @slot value | ||
| * @slot icon |
There was a problem hiding this comment.
It looks like the descriptions for these were lost in the process
| } | ||
|
|
||
| :host([hidden]) { | ||
| :host(:where([hide], :state(hidden))) { |
There was a problem hiding this comment.
This selector can stay as it was, there is not :state(hidden) and the attribute got renamed incorrectly anyway. This is here to support the native hidden attribute on custom elements.
| function createFixture({ empty = false, ellipsis = false, inline = false } = {}): Promise<ILabelValueComponent> { | ||
| return fixture<ILabelValueComponent>(html` | ||
| <forge-label-value ?empty=${empty} ?ellipsis=${ellipsis} ?inline=${inline}> | ||
| <span slot="label">Label</span> | ||
| <span slot="value">Value</span> | ||
| </forge-label-value> | ||
| `); | ||
| } |
There was a problem hiding this comment.
Why was this removed? It's important that test the component with the slots, some of the other tests above aren't actually rendering content.
Could you revert these changes, and instead only add new assertions for the CSS states?
| if (changedProperties.has('dense')) { | ||
| if (this.hasUpdated || this.hasAttribute('dense')) { | ||
| // Ignore the initialization cycle. On subsequent updates, map dense to inline | ||
| if (this.inline !== this.dense) { | ||
| this.inline = this.dense; | ||
| } | ||
| } |
There was a problem hiding this comment.
I know it's unfortunate that we have to keep these properties in sync for now to avoid breaking changes, but I'm not sure how I feel about this block. I also noticed that the dense attribute does not get removed if setting inline to false. Not really a big deal since typically only one of these properties is in use at a time, but better to handle it gracefully. Maybe we can clean this up and make it more simple/readable?
3 participants
PR Checklist
Please check if your PR fulfills the following requirements:
Describe the new behavior?
Refactored the "Label Value" component to use Lit
Additional information
Retained deprecated "dense" prop, if used will map to "inline" via willUpdate() call