feat(slider): add displaysPercentageColors prop
#3804
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a public boolean prop Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/slider/examples/slider-multiplier-percentage-colors.tsx (1)
30-57: 🛠️ Refactor suggestion | 🟠 MajorWrap multiple elements in
<Host>instead of an array literal.The render method should not return an array. Wrap the elements in
<Host>and add the import from@stencil/core.♻️ Proposed refactor
-import { Component, h, State } from '@stencil/core'; +import { Component, h, Host, State } from '@stencil/core'; @@ public render() { - return [ - <limel-slider - displaysPercentageColors={true} - label="Slider with percentage colors" - unit=" %" - value={this.value} - factor={this.factor} - valuemax={this.maxValue} - valuemin={this.minValue} - disabled={this.disabled} - readonly={this.readonly} - onChange={this.changeHandler} - />, - <limel-example-controls> - <limel-switch - value={this.disabled} - label="Disabled" - onChange={this.setDisabled} - /> - <limel-switch - value={this.readonly} - label="Readonly" - onChange={this.setReadonly} - /> - </limel-example-controls>, - <limel-example-value value={this.value} />, - ]; + return ( + <Host> + <limel-slider + displaysPercentageColors={true} + label="Slider with percentage colors" + unit=" %" + value={this.value} + factor={this.factor} + valuemax={this.maxValue} + valuemin={this.minValue} + disabled={this.disabled} + readonly={this.readonly} + onChange={this.changeHandler} + /> + <limel-example-controls> + <limel-switch + value={this.disabled} + label="Disabled" + onChange={this.setDisabled} + /> + <limel-switch + value={this.readonly} + label="Readonly" + onChange={this.setReadonly} + /> + </limel-example-controls> + <limel-example-value value={this.value} /> + </Host> + ); }
adrianschmidt
force-pushed
the
feat/slider-percentage-colors-prop
branch
from
760999e to
48325b3
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/slider/examples/slider-multiplier-percentage-colors.tsx (1)
30-57: 🧹 Nitpick | 🔵 TrivialConsider wrapping multiple JSX elements in
<Host>instead of an array literal.The
render()method returns an array literal with multiple JSX elements. As per coding guidelines for StencilJS components, multiple top-level JSX elements should be wrapped in the special<Host>element rather than an array literal.♻️ Proposed refactor
public render() { - return [ + return ( + <Host> <limel-slider displaysPercentageColors={true} label="Slider with percentage colors" unit=" %" value={this.value} factor={this.factor} valuemax={this.maxValue} valuemin={this.minValue} disabled={this.disabled} readonly={this.readonly} onChange={this.changeHandler} - />, + /> <limel-example-controls> <limel-switch value={this.disabled} label="Disabled" onChange={this.setDisabled} /> <limel-switch value={this.readonly} label="Readonly" onChange={this.setReadonly} /> - </limel-example-controls>, - <limel-example-value value={this.value} />, - ]; + </limel-example-controls> + <limel-example-value value={this.value} /> + </Host> + ); }Also add
Hostto the import:-import { Component, h, State } from '@stencil/core'; +import { Component, h, Host, State } from '@stencil/core';
Head branch was pushed to by a user without write access
adrianschmidt-bot
force-pushed
the
feat/slider-percentage-colors-prop
branch
from
48325b3 to
d237e29
Compare
adrianschmidt
force-pushed
the
feat/slider-percentage-colors-prop
branch
from
d237e29 to
47f71e0
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/components/slider/examples/slider-multiplier-percentage-colors.tsx (1)
30-57: 🛠️ Refactor suggestion | 🟠 MajorWrap multiple top-level JSX elements in
<Host>instead of an array literal.The render method returns an array literal, but the coding guidelines require using Stencil's
<Host>element for multiple top-level nodes. AddHostto the import from@stencil/coreand wrap the elements accordingly.♻️ Proposed refactor
-import { Component, h, State } from '@stencil/core'; +import { Component, h, Host, State } from '@stencil/core'; public render() { - return [ - <limel-slider - displaysPercentageColors={true} - label="Slider with percentage colors" - unit=" %" - value={this.value} - factor={this.factor} - valuemax={this.maxValue} - valuemin={this.minValue} - disabled={this.disabled} - readonly={this.readonly} - onChange={this.changeHandler} - />, - <limel-example-controls> - <limel-switch - value={this.disabled} - label="Disabled" - onChange={this.setDisabled} - /> - <limel-switch - value={this.readonly} - label="Readonly" - onChange={this.setReadonly} - /> - </limel-example-controls>, - <limel-example-value value={this.value} />, - ]; + return ( + <Host> + <limel-slider + displaysPercentageColors={true} + label="Slider with percentage colors" + unit=" %" + value={this.value} + factor={this.factor} + valuemax={this.maxValue} + valuemin={this.minValue} + disabled={this.disabled} + readonly={this.readonly} + onChange={this.changeHandler} + /> + <limel-example-controls> + <limel-switch + value={this.disabled} + label="Disabled" + onChange={this.setDisabled} + /> + <limel-switch + value={this.readonly} + label="Readonly" + onChange={this.setReadonly} + /> + </limel-example-controls> + <limel-example-value value={this.value} /> + </Host> + ); }
adrianschmidt-bot
force-pushed
the
feat/slider-percentage-colors-prop
branch
from
47f71e0 to
64d950a
Compare
Replace the need to manually add the `displays-percentage-colors` class with a proper component prop. This makes the feature discoverable through TypeScript autocompletion and component documentation. The class-based approach still works for backward compatibility since the CSS rules check for the class on the host element. Fixes Lundalogik#3694
adrianschmidt
force-pushed
the
feat/slider-percentage-colors-prop
branch
from
64d950a to
862ccf4
Compare
|
Documentation has been published to https://lundalogik.github.io/lime-elements/versions/PR-3804/ |
|
🎉 This PR is included in version 38.44.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
3 participants
Summary
Replace the need to manually add the
displays-percentage-colorsclass with a proper component prop. This makes the feature discoverable through TypeScript autocompletion and component documentation.The class-based approach still works for backward compatibility since the CSS rules check for the class on the host element.
Fixes #3694
Changes
displaysPercentageColorsprop tolimel-slidergetContainerClassList()to apply the class when prop istrueReview:
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.