Redesign how the control bar handles fill and stroke colors (#4137)

* Revamp how the control bar handles fill and stroke colors

* Fix bugs

* Code review
This commit is contained in:
Keavon Chambers
2026-05-11 18:13:02 -07:00
committed by GitHub
parent f6def3b911
commit 629a1f4b4c
25 changed files with 1355 additions and 549 deletions

View File

@@ -328,7 +328,7 @@
.icon-button,
.text-button,
.popover-button,
.color-button > button,
.color-input > button,
.color-picker .preset-color,
.working-colors-input .swatch > button,
.radio-input button,

View File

@@ -215,6 +215,7 @@
$$events: {
value: (e: CustomEvent) => widgetValueUpdate(index, e.detail, true),
startHistoryTransaction: () => widgetValueCommit(index, props.value),
commitHistoryTransaction: () => editor.endTransaction(),
},
}),
},

View File

@@ -13,6 +13,7 @@
export let icon: IconName | undefined = undefined;
export let forLabel: bigint | undefined = undefined;
export let disabled = false;
export let mixed = false;
// Tooltips
export let tooltipLabel: string | undefined = undefined;
export let tooltipDescription: string | undefined = undefined;
@@ -22,6 +23,7 @@
$: id = forLabel !== undefined ? String(forLabel) : backupId;
$: displayIcon = !checked && (!icon || icon === "Checkmark") ? "Empty12px" : icon || "Checkmark";
$: if (inputElement) inputElement.indeterminate = mixed;
export function isChecked() {
return checked;
@@ -43,7 +45,14 @@
type="checkbox"
id={`checkbox-input-${id}`}
bind:checked
on:change={(_) => dispatch("checked", inputElement?.checked || false)}
on:change={(_) => {
// Clicking a mixed-state checkbox always transitions to ticked rather than following HTML's default toggle from the previous `checked` value
if (mixed && inputElement && !inputElement.checked) {
inputElement.checked = true;
checked = true;
}
dispatch("checked", inputElement?.checked || false);
}}
{disabled}
tabindex={disabled ? -1 : 0}
bind:this={inputElement}
@@ -51,6 +60,7 @@
<label
class:disabled
class:checked
class:mixed
for={`checkbox-input-${id}`}
on:keydown={(e) => e.key === "Enter" && toggleCheckboxFromLabel(e)}
data-tooltip-label={tooltipLabel}
@@ -130,6 +140,29 @@
}
}
// Mixed (takes priority over both checked and unchecked appearances)
label.mixed .checkbox-box,
input:checked + label.mixed .checkbox-box {
position: relative;
background: var(--color-5-dullgray);
.icon-label {
visibility: hidden;
}
&::after {
content: "";
position: absolute;
top: 50%;
left: 50%;
width: 8px;
height: 2px;
transform: translate(-50%, -50%);
background: var(--color-8-uppergray);
border-radius: 1px;
}
}
+ .text-label.text-label {
margin-left: 8px;
}

View File

@@ -14,8 +14,12 @@
// export let allowTransparency = false; // TODO: Implement
export let menuDirection: MenuDirection = "Bottom";
export let disabled = false;
export let mixed = false;
// Styling
export let narrow = false;
// Sizing
export let minWidth = 0;
export let maxWidth = 0;
// Tooltips
export let tooltipLabel: string | undefined = undefined;
export let tooltipDescription: string | undefined = undefined;
@@ -23,7 +27,7 @@
let open = false;
$: outlineFactor = contrastingOutlineFactor(value, ["--color-1-nearblack", "--color-3-darkgray"], 0.01);
$: outlineFactor = contrastingOutlineFactor(value, "--color-3-darkgray", 0.01);
$: outlined = outlineFactor > 0.0001;
$: gradientStops = fillChoiceGradientStops(value);
$: solidColor = fillChoiceColor(value);
@@ -31,7 +35,17 @@
$: transparency = gradientStops ? gradientStops.color.some((color) => color.alpha < 1) : solidColor ? solidColor.alpha < 1 : false;
</script>
<LayoutCol class="color-button" classes={{ open, disabled, narrow, none, transparency, outlined, "direction-top": menuDirection === "Top" }} {tooltipLabel} {tooltipDescription} {tooltipShortcut}>
<LayoutCol
class="color-input"
classes={{ open, disabled, narrow, none, transparency, outlined, mixed, "direction-top": menuDirection === "Top" }}
styles={{
...(minWidth > 0 ? { "min-width": `${minWidth}px` } : {}),
...(maxWidth > 0 ? { "max-width": `${maxWidth}px` } : {}),
}}
{tooltipLabel}
{tooltipDescription}
{tooltipShortcut}
>
<button style:--chosen-gradient={chosenGradient} style:--outline-amount={outlineFactor} on:click={() => (open = true)} tabindex="0" data-floating-menu-spawner></button>
<ColorPicker
{open}
@@ -53,7 +67,7 @@
</LayoutCol>
<style lang="scss">
.color-button {
.color-input {
position: relative;
min-width: 80px;
@@ -131,6 +145,28 @@
opacity: 0.5;
}
&.mixed > button {
position: relative;
background: var(--color-e-nearwhite);
background-image: none;
&::before {
background: var(--color-e-nearwhite);
}
&::after {
content: "";
position: absolute;
top: 50%;
left: 50%;
width: 8px;
height: 2px;
border-radius: 1px;
transform: translate(-50%, -50%);
background: var(--color-8-uppergray);
}
}
&:not(.disabled):hover > button .text-label,
&:not(.disabled).open > button .text-label {
background: var(--color-6-lowergray);

View File

@@ -12,7 +12,7 @@
const BUTTON_LEFT = 0;
const BUTTON_RIGHT = 2;
const dispatch = createEventDispatcher<{ value: number | undefined; startHistoryTransaction: undefined }>();
const dispatch = createEventDispatcher<{ value: number | undefined; startHistoryTransaction: undefined; commitHistoryTransaction: undefined }>();
const editor = getContext<EditorWrapper>("editor");
@@ -84,6 +84,9 @@
let shiftKeyDown = false;
// Track whether the Ctrl key is currently held down.
let ctrlKeyDown = false;
// True between dispatching `startHistoryTransaction` and the matching `commitHistoryTransaction`, so we only commit
// when this widget actually opened a transaction (skipping clicks-without-drag and aborts-before-drag-started).
let transactionInProgress = false;
// Cleanup function for active drag interactions, called on destroy to prevent leaked listeners
let activeDragCleanup: (() => void) | undefined;
// Track the slider abort state for cleanup on destroy
@@ -135,8 +138,17 @@
removeEventListener("keydown", sliderAbortFromDragging);
removeEventListener("keydown", incrementPressAbort);
if (sliderResetAbortHandler) removeEventListener("pointerup", sliderResetAbortHandler);
commitTransactionIfInProgress();
});
function commitTransactionIfInProgress() {
if (transactionInProgress) {
dispatch("commitHistoryTransaction");
transactionInProgress = false;
}
}
// ===============================
// TRACKING AND UPDATING THE VALUE
// ===============================
@@ -243,9 +255,13 @@
if (newValue !== undefined) {
const oldValue = value !== undefined && isInteger ? Math.round(value) : value;
if (newValue !== oldValue) dispatch("startHistoryTransaction");
if (newValue !== oldValue) {
dispatch("startHistoryTransaction");
transactionInProgress = true;
}
}
updateValue(newValue);
commitTransactionIfInProgress();
editing = false;
self?.unFocus();
@@ -477,6 +493,9 @@
// Clean up the event listeners.
activeDragCleanup?.();
// Close out the transaction `startDragging` opened so the many emits collapse into one history step (covers both confirmed and aborted drags).
commitTransactionIfInProgress();
};
addEventListener("pointerup", pointerUp);
@@ -626,12 +645,17 @@
removeEventListener("keydown", sliderAbortFromMousedown);
removeEventListener("pointermove", sliderAbortFromDragging);
removeEventListener("keydown", sliderAbortFromDragging);
// Close out the transaction `startDragging` opened, so the drag's many emits collapse into one history step.
// Covers the abort path too (sliderAbort already restored the original value, so the committed step is a no-op).
commitTransactionIfInProgress();
}
function startDragging() {
// This event is sent to the backend so it knows to start a transaction for the history system. See discussion for some explanation:
// <https://github.com/GraphiteEditor/Graphite/pull/1584#discussion_r1477592483>
dispatch("startHistoryTransaction");
transactionInProgress = true;
}
// We want to let the user abort while dragging the slider by right clicking or pressing Escape.
@@ -663,6 +687,8 @@
// dragging the slider, now that we're no longer dragging it due to the loss of window focus.
removeEventListener("pointermove", sliderAbortFromDragging);
removeEventListener("keydown", sliderAbortFromDragging);
commitTransactionIfInProgress();
}
}

View File

@@ -343,7 +343,12 @@ function detectShake(e: PointerEvent | MouseEvent): boolean {
}
function targetIsTextField(target: EventTarget | HTMLElement | undefined): boolean {
return target instanceof HTMLElement && (target.nodeName === "INPUT" || target.nodeName === "TEXTAREA" || target.isContentEditable);
if (!(target instanceof HTMLElement)) return false;
return (
target.isContentEditable ||
target instanceof HTMLTextAreaElement ||
(target instanceof HTMLInputElement && ["text", "password", "email", "url", "tel", "search", "number", "date", "datetime-local", "month", "time", "week"].includes(target.type))
);
}
function potentiallyRestoreCanvasFocus(e: Event) {

View File

@@ -334,9 +334,18 @@ impl EditorWrapper {
pub fn widget_value_commit_and_update(&self, layout_target: JsValue, widget_id: u64, value: JsValue, resend_widget: bool) -> Result<(), JsValue> {
self.widget_value_commit_helper(layout_target.clone(), widget_id, value.clone())?;
self.widget_value_update_helper(layout_target, widget_id, value, resend_widget)?;
// Close out a transaction that the widget's `on_commit` opened (if any), so a single click on widgets like the
// NumberInput's increment buttons collapses into one history step instead of leaving the transaction in `Modified`
self.dispatch(DocumentMessage::EndTransaction);
Ok(())
}
/// Closes out the current transaction (drag-end / text-commit end), so emits during a slider drag collapse into one history step instead of N
#[wasm_bindgen(js_name = endTransaction)]
pub fn end_transaction(&self) {
self.dispatch(DocumentMessage::EndTransaction);
}
pub fn widget_value_update_helper(&self, layout_target: JsValue, widget_id: u64, value: JsValue, resend_widget: bool) -> Result<(), JsValue> {
let widget_id = WidgetId(widget_id);
match (from_value(layout_target), from_value(value)) {