Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Don't update saturation based on parsed color #536

Merged
merged 23 commits into from
Feb 5, 2024
Merged
Show file tree
Hide file tree
Changes from 20 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8397ad4
Don't update saturation based on parsed color
bjarnef Jul 18, 2023
a716476
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Jul 24, 2023
87f951a
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Sep 28, 2023
9fe8a29
Workaround to update color area when hue slider change
bjarnef Sep 28, 2023
35e7860
Fix typo
bjarnef Sep 28, 2023
dd34062
Don't change opacity of gradient in opacity-slider as value change an…
bjarnef Sep 28, 2023
6468c60
Make handle opacity based on alpha
bjarnef Sep 28, 2023
ea6af27
Clear console
bjarnef Sep 29, 2023
2570972
Clear console
bjarnef Sep 29, 2023
5eec6bd
Clear console
bjarnef Sep 29, 2023
77774cb
Add comment
bjarnef Sep 29, 2023
75bfabf
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Sep 29, 2023
6d26f02
Remove output of hue value
bjarnef Sep 29, 2023
a1dd812
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Oct 14, 2023
03e3b72
Update saturation
bjarnef Oct 14, 2023
3bdf47a
Don't update saturation
bjarnef Oct 14, 2023
68b0de2
Merge branch 'v1/contrib' into bug/color-area-saturation
iOvergaard Oct 17, 2023
06e324c
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Oct 18, 2023
ade0a0f
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Nov 8, 2023
7d803bb
Merge branch 'v1/contrib' into bug/color-area-saturation
bjarnef Feb 4, 2024
429f299
Remove commented line
bjarnef Feb 5, 2024
49e54c5
Remove unused parameter
bjarnef Feb 5, 2024
f9598d5
Run linter
bjarnef Feb 5, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 8 additions & 3 deletions packages/uui-color-area/lib/uui-color-area.element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,12 +97,17 @@
const parsed = colord(newVal);

if (parsed.isValid()) {
const { h, s, l } = parsed.toHsl();
const { h, s, l, a } = parsed.toHsl();

Check failure on line 100 in packages/uui-color-area/lib/uui-color-area.element.ts

View workflow job for this annotation

GitHub Actions / test (20)

's' is assigned a value but never used

this.hue = h;
this.saturation = s;
// Update hue from parsed color, but when color is black, value from hue slider may be different from zero.
if (h !== 0) {
this.hue = h;
}

//this.saturation = s;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried the following here, but it seem it made the handle "jump" and bit anyway and the color spectrum seemed to be affected. When dragging the handle in the spectrum, I would expect the gradient shouldn't change.

if (s !== 0) {
    this.saturation= s;
}

this.lightness = l;
this.brightness = this.getBrightness(l);
this.alpha = a * 100;
}
} catch (e) {
// TODO: Should we log this?
Expand Down
34 changes: 31 additions & 3 deletions packages/uui-color-picker/lib/uui-color-picker.element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -360,15 +360,25 @@ export class UUIColorPickerElement extends LabelMixin('label', LitElement) {
}

setColor(colorString: string | HslaColor) {
this._colord = new Colord(colorString);

const { h, l, s, a } = this._colord.toHsl();
const colord = new Colord(colorString);

const { h, s, l, a } = colord.toHsl();

this.hue = h;
this.saturation = s;
this.lightness = l;
this.alpha = this.opacity ? a * 100 : 100;

const hslaColor = colorString as HslaColor;

// Workaround as hue isn't correct after changing hue slider, but Colord parse hue value as zero when color is black.
if (hslaColor && hslaColor.h) {
this.hue = hslaColor.h;
}
iOvergaard marked this conversation as resolved.
Show resolved Hide resolved

this._colord = colord;

this._syncValues();

this.dispatchEvent(
Expand All @@ -384,6 +394,23 @@ export class UUIColorPickerElement extends LabelMixin('label', LitElement) {
return this.uppercase ? string.toUpperCase() : string.toLowerCase();
}

/** Generates a hex string from HSL values. Hue must be 0-360. All other arguments must be 0-100. */
private getHexString(
hue: number,
saturation: number,
lightness: number,
alpha = 100
) {
const color = colord(
`hsla(${hue}, ${saturation}%, ${lightness}%, ${alpha / 100})`
);
if (!color.isValid()) {
return '';
}

return color.toHex();
}
bjarnef marked this conversation as resolved.
Show resolved Hide resolved

private _syncValues() {
this.inputValue = this.getFormattedValue(this.format);
this.value = this.inputValue;
Expand All @@ -400,6 +427,7 @@ export class UUIColorPickerElement extends LabelMixin('label', LitElement) {
aria-disabled=${this.disabled ? 'true' : 'false'}>
<uui-color-area
.value="${this.value}"
.hue="${Math.round(this.hue)}"
?disabled=${this.disabled}
@change=${this.handleGridChange}>
</uui-color-area>
Expand All @@ -419,7 +447,7 @@ export class UUIColorPickerElement extends LabelMixin('label', LitElement) {
class="opacity-slider"
.value=${Math.round(this.alpha)}
type="opacity"
.color=${this.value}
.color=${this.getHexString(this.hue, this.saturation, this.lightness)}
?disabled=${this.disabled}
@change=${this.handleAlphaChange}>
</uui-color-slider>
Expand Down
4 changes: 2 additions & 2 deletions packages/uui-color-slider/lib/uui-color-slider.element.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,7 @@ export class UUIColorSliderElement extends LabelMixin('label', LitElement) {
this.dispatchEvent(new UUIColorSliderEvent(UUIColorSliderEvent.CHANGE));
}

get ccsPropCurrentValue() {
get cssPropCurrentValue() {
return this.value === 0
? this.vertical
? 100
Expand Down Expand Up @@ -257,7 +257,7 @@ export class UUIColorSliderElement extends LabelMixin('label', LitElement) {
<!-- <slot name="detail"> </slot> -->
<span
id="color-slider__handle"
style="--current-value: ${this.ccsPropCurrentValue}%;"
style="--current-value: ${this.cssPropCurrentValue}%;"
tabindex=${ifDefined(this.disabled ? undefined : '0')}></span>
</div>
${Math.round(this.value)}`;
Expand Down
Loading