Skip to content

Commit f19bcad

Browse files
committed
Fix and polish an assortment of small issues
1 parent 8f3d82c commit f19bcad

File tree

13 files changed

+96
-66
lines changed

13 files changed

+96
-66
lines changed

editor/src/messages/dialog/preferences_dialog/preferences_dialog_message_handler.rs

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
use crate::consts::{UI_SCALE_DEFAULT, UI_SCALE_MAX, UI_SCALE_MIN, VIEWPORT_ZOOM_WHEEL_RATE, VIEWPORT_ZOOM_WHEEL_RATE_CHANGE};
1+
use crate::consts::{VIEWPORT_ZOOM_WHEEL_RATE, VIEWPORT_ZOOM_WHEEL_RATE_CHANGE};
22
use crate::messages::layout::utility_types::widget_prelude::*;
33
use crate::messages::portfolio::document::utility_types::wires::GraphWireStyle;
44
use crate::messages::preferences::SelectionMode;
@@ -155,14 +155,14 @@ impl PreferencesDialogMessageHandler {
155155
rows.extend_from_slice(&[header, selection_label, selection_mode]);
156156
}
157157

158-
// ==========
159-
// UI
160-
// ==========
158+
// =========
159+
// INTERFACE
160+
// =========
161161
#[cfg(not(target_family = "wasm"))]
162162
{
163-
let header = vec![TextLabel::new("UI").italic(true).widget_instance()];
163+
let header = vec![TextLabel::new("Interface").italic(true).widget_instance()];
164164

165-
let scale_description = "Adjust the scale of the user interface (100 is default).";
165+
let scale_description = "Adjust the scale of the entire user interface (100% is default).";
166166
let scale_label = vec![
167167
Separator::new(SeparatorType::Unrelated).widget_instance(),
168168
Separator::new(SeparatorType::Unrelated).widget_instance(),
@@ -176,15 +176,18 @@ impl PreferencesDialogMessageHandler {
176176
.tooltip_description(scale_description)
177177
.mode_range()
178178
.int()
179-
.min(ui_scale_to_display(UI_SCALE_MIN))
180-
.max(ui_scale_to_display(UI_SCALE_MAX))
179+
.min(ui_scale_to_display(crate::consts::UI_SCALE_MIN))
180+
.max(ui_scale_to_display(crate::consts::UI_SCALE_MAX))
181181
.unit("%")
182182
.on_update(|number_input: &NumberInput| {
183183
if let Some(display_value) = number_input.value {
184184
let scale = map_display_to_ui_scale(display_value);
185185
PreferencesMessage::UIScale { scale }.into()
186186
} else {
187-
PreferencesMessage::UIScale { scale: UI_SCALE_DEFAULT }.into()
187+
PreferencesMessage::UIScale {
188+
scale: crate::consts::UI_SCALE_DEFAULT,
189+
}
190+
.into()
188191
}
189192
})
190193
.widget_instance(),
@@ -376,11 +379,13 @@ fn map_zoom_rate_to_display(rate: f64) -> f64 {
376379
}
377380

378381
/// Maps display values in percent to actual ui scale.
382+
#[cfg(not(target_family = "wasm"))]
379383
fn map_display_to_ui_scale(display: f64) -> f64 {
380384
display / 100.
381385
}
382386

383387
/// Maps actual ui scale back to display values in percent.
388+
#[cfg(not(target_family = "wasm"))]
384389
fn ui_scale_to_display(scale: f64) -> f64 {
385390
scale * 100.
386391
}

editor/src/messages/portfolio/document/document_message.rs

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -49,8 +49,6 @@ pub enum DocumentMessage {
4949
},
5050
DeleteSelectedLayers,
5151
DeselectAllLayers,
52-
DocumentHistoryBackward,
53-
DocumentHistoryForward,
5452
DocumentStructureChanged,
5553
DrawArtboardOverlays {
5654
context: OverlayContext,
@@ -110,7 +108,6 @@ pub enum DocumentMessage {
110108
mouse: Option<(f64, f64)>,
111109
parent_and_insert_index: Option<(LayerNodeIdentifier, usize)>,
112110
},
113-
Redo,
114111
RenameDocument {
115112
new_name: String,
116113
},
@@ -179,12 +176,27 @@ pub enum DocumentMessage {
179176
SetRenderMode {
180177
render_mode: RenderMode,
181178
},
179+
Undo,
180+
Redo,
181+
DocumentHistoryBackward,
182+
DocumentHistoryForward,
183+
// TODO: Rename to HistoryStepPush
184+
/// Create a snapshot of the document at this point in time, by immediately starting and committing a transaction.
182185
AddTransaction,
186+
// TODO: Rename to HistoryTransactionStart
187+
/// Take a snapshot of the document to an intermediate state, and then depending on what we do next, we might either commit or abort it.
183188
StartTransaction,
189+
// TODO: Rename to HistoryTransactionEnd
190+
/// Either commit (creating a new history step) or cancel (removing the last history step, as if it never happened) the last transaction started with `StartTransaction`.
184191
EndTransaction,
192+
// TODO: Remove this, make it into a private function
185193
CommitTransaction,
194+
// TODO: Remove this, make it into a private function
186195
CancelTransaction,
196+
/// Cause the document to revert back to the state when the transaction was started. For example, the user may be dragging
197+
/// something around and hits Escape to abort the drag. This jumps the document back to the point before the drag began.
187198
AbortTransaction,
199+
/// The same as `AbortTransaction` with one step back, but it can also be called with multiple steps back in the history of undos.
188200
RepeatedAbortTransaction {
189201
undo_count: usize,
190202
},
@@ -208,7 +220,6 @@ pub enum DocumentMessage {
208220
UpdateClipTargets {
209221
clip_targets: HashSet<NodeId>,
210222
},
211-
Undo,
212223
UngroupSelectedLayers,
213224
UngroupLayer {
214225
layer: LayerNodeIdentifier,

editor/src/messages/portfolio/document/document_message_handler.rs

Lines changed: 49 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -372,8 +372,6 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
372372
responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![] });
373373
self.layer_range_selection_reference = None;
374374
}
375-
DocumentMessage::DocumentHistoryBackward => self.undo_with_history(viewport, responses),
376-
DocumentMessage::DocumentHistoryForward => self.redo_with_history(viewport, responses),
377375
DocumentMessage::DocumentStructureChanged => {
378376
if layers_panel_open {
379377
self.network_interface.load_structure();
@@ -953,15 +951,6 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
953951
responses.add(NodeGraphMessage::SelectedNodesSet { nodes: vec![layer.to_node()] });
954952
responses.add(ToolMessage::ActivateTool { tool_type: ToolType::Select });
955953
}
956-
DocumentMessage::Redo => {
957-
if self.network_interface.transaction_status() != TransactionStatus::Finished {
958-
return;
959-
}
960-
responses.add(SelectToolMessage::Abort);
961-
responses.add(DocumentMessage::DocumentHistoryForward);
962-
responses.add(ToolMessage::Redo);
963-
responses.add(OverlaysMessage::Draw);
964-
}
965954
DocumentMessage::RenameDocument { new_name } => {
966955
self.name = new_name.clone();
967956

@@ -1291,6 +1280,27 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
12911280
self.render_mode = render_mode;
12921281
responses.add_front(NodeGraphMessage::RunDocumentGraph);
12931282
}
1283+
DocumentMessage::Undo => {
1284+
if self.network_interface.transaction_status() != TransactionStatus::Finished {
1285+
return;
1286+
}
1287+
responses.add(ToolMessage::PreUndo);
1288+
responses.add(DocumentMessage::DocumentHistoryBackward);
1289+
responses.add(OverlaysMessage::Draw);
1290+
responses.add(ToolMessage::Undo);
1291+
}
1292+
DocumentMessage::Redo => {
1293+
if self.network_interface.transaction_status() != TransactionStatus::Finished {
1294+
return;
1295+
}
1296+
responses.add(SelectToolMessage::Abort);
1297+
responses.add(DocumentMessage::DocumentHistoryForward);
1298+
responses.add(ToolMessage::Redo);
1299+
responses.add(OverlaysMessage::Draw);
1300+
}
1301+
DocumentMessage::DocumentHistoryBackward => self.undo_with_history(viewport, responses),
1302+
DocumentMessage::DocumentHistoryForward => self.redo_with_history(viewport, responses),
1303+
// Create a snapshot of the document at this point in time, by immediately starting and committing a transaction.
12941304
DocumentMessage::AddTransaction => {
12951305
// Reverse order since they are added to the front
12961306
responses.add_front(DocumentMessage::CommitTransaction);
@@ -1307,20 +1317,18 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
13071317
// Push the UpdateOpenDocumentsList message to the bus in order to update the save status of the open documents
13081318
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
13091319
}
1310-
// Commits the transaction if the network was mutated since the transaction started, otherwise it cancels the transaction
1320+
// Either commit (creating a new history step) or cancel (removing the last history step, as if it never happened) the last transaction started with `StartTransaction`.
13111321
DocumentMessage::EndTransaction => match self.network_interface.transaction_status() {
1312-
TransactionStatus::Started => {
1313-
responses.add_front(DocumentMessage::CancelTransaction);
1314-
}
1315-
TransactionStatus::Modified => {
1316-
responses.add_front(DocumentMessage::CommitTransaction);
1317-
}
1322+
// This is used if, between the start and end of the transaction, the changes were undone by the user.
1323+
// For example, dragging something around and then dropping it back at its exact original position.
1324+
// So we cancel the transaction to return to the point before the transaction was started.
1325+
TransactionStatus::Started => responses.add_front(DocumentMessage::CancelTransaction),
1326+
// This is used if, between the start and end of the transaction, actual changes did occur and we want to keep them as part of a history step that the user can undo/redo.
1327+
TransactionStatus::Modified => responses.add_front(DocumentMessage::CommitTransaction),
1328+
// This is an erroneous state indicating that a transaction is being ended without having ever been started.
13181329
TransactionStatus::Finished => {}
13191330
},
1320-
DocumentMessage::CancelTransaction => {
1321-
self.network_interface.finish_transaction();
1322-
self.document_undo_history.pop_back();
1323-
}
1331+
// Add a history step so the user can undo/redo to the point when the transaction was started.
13241332
DocumentMessage::CommitTransaction => {
13251333
if self.network_interface.transaction_status() == TransactionStatus::Finished {
13261334
return;
@@ -1329,25 +1337,38 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
13291337
self.document_redo_history.clear();
13301338
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
13311339
}
1340+
// Retroactively undoes the start of the transaction, as if the transaction was never started. This is useful, for example, if the user
1341+
// might have begun performing some action that ends up not changing anything, like dragging something back to its exact original position.
1342+
DocumentMessage::CancelTransaction => {
1343+
self.network_interface.finish_transaction();
1344+
self.document_undo_history.pop_back();
1345+
}
1346+
// Cause the document to revert back to the state when the transaction was started. For example, the user may be dragging
1347+
// something around and hits Escape to abort the drag. This jumps the document back to the point before the drag began.
13321348
DocumentMessage::AbortTransaction => match self.network_interface.transaction_status() {
1333-
TransactionStatus::Started => {
1334-
responses.add_front(DocumentMessage::CancelTransaction);
1335-
}
1336-
TransactionStatus::Modified => {
1337-
responses.add(DocumentMessage::RepeatedAbortTransaction { undo_count: 1 });
1338-
}
1349+
// If we abort a transaction without any changes having been made, we simply remove the transaction as if it never occurred.
1350+
TransactionStatus::Started => responses.add_front(DocumentMessage::CancelTransaction),
1351+
// If we abort a transaction after changes have been made, we need to undo those changes.
1352+
TransactionStatus::Modified => responses.add(DocumentMessage::RepeatedAbortTransaction { undo_count: 1 }),
1353+
// This is an erroneous state indicating that a transaction is being aborted without having ever been started.
13391354
TransactionStatus::Finished => {}
13401355
},
1356+
// The same as `AbortTransaction` with one step back, but it can also be called with multiple steps back in the history of undos.
13411357
DocumentMessage::RepeatedAbortTransaction { undo_count } => {
1358+
// This prevents us from aborting a transaction multiple times in a row, which would be erroneous.
13421359
if self.network_interface.transaction_status() == TransactionStatus::Finished {
13431360
return;
13441361
}
13451362

1363+
// Sometimes (like successive G/R/S transformations) we may need to undo multiple steps to fully abort the transaction, before we finish.
13461364
for _ in 0..undo_count {
13471365
self.undo(viewport, responses);
13481366
}
13491367

1368+
// Finally finish the transaction, ensuring that any future operations are not erroneously redone as part of this aborted transaction.
13501369
self.network_interface.finish_transaction();
1370+
1371+
// Refresh state
13511372
responses.add(OverlaysMessage::Draw);
13521373
responses.add(PortfolioMessage::UpdateOpenDocumentsList);
13531374
}
@@ -1419,15 +1440,6 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
14191440
DocumentMessage::UpdateClipTargets { clip_targets } => {
14201441
self.network_interface.update_clip_targets(clip_targets);
14211442
}
1422-
DocumentMessage::Undo => {
1423-
if self.network_interface.transaction_status() != TransactionStatus::Finished {
1424-
return;
1425-
}
1426-
responses.add(ToolMessage::PreUndo);
1427-
responses.add(DocumentMessage::DocumentHistoryBackward);
1428-
responses.add(OverlaysMessage::Draw);
1429-
responses.add(ToolMessage::Undo);
1430-
}
14311443
DocumentMessage::UngroupSelectedLayers => {
14321444
if !self.selection_network_path.is_empty() {
14331445
log::error!("Ungrouping selected layers is only supported for the Document Network");

editor/src/messages/preferences/preferences_message_handler.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ use crate::messages::prelude::*;
66
use graph_craft::wasm_application_io::EditorPreferences;
77

88
#[derive(Debug, PartialEq, Clone, serde::Serialize, serde::Deserialize, specta::Type, ExtractField)]
9+
#[serde(default)]
910
pub struct PreferencesMessageHandler {
1011
pub selection_mode: SelectionMode,
1112
pub zoom_with_scroll: bool,

editor/src/messages/tool/common_functionality/pivot.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -169,6 +169,7 @@ pub struct PivotGizmoState {
169169

170170
impl PivotGizmoState {
171171
pub fn is_pivot_type(&self) -> bool {
172+
// A disabled pivot is considered a pivot-type gizmo that is always centered
172173
self.gizmo_type == PivotGizmoType::Pivot || self.disabled
173174
}
174175

frontend/src/components/layout/FloatingMenu.svelte

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -372,6 +372,9 @@
372372
// Start with the parent of the spawner for this floating menu and keep widening the search for any other valid spawners that are hover-transferrable
373373
let currentAncestor = (targetSpawner && ownSpawner?.parentElement) || undefined;
374374
while (currentAncestor) {
375+
// If the current ancestor blocks hover transfer, stop searching
376+
if (currentAncestor.hasAttribute("data-block-hover-transfer")) break;
377+
375378
const ownSpawnerDepthFromCurrentAncestor = ownSpawner && getDepthFromAncestor(ownSpawner, currentAncestor);
376379
const currentAncestor2 = currentAncestor; // This duplicate variable avoids an ESLint warning
377380
@@ -382,8 +385,8 @@
382385
const notOurself = !ownDescendantMenuSpawners.includes(item);
383386
// And filter away unequal depths from the current ancestor
384387
const notUnequalDepths = notOurself && getDepthFromAncestor(item, currentAncestor2) === ownSpawnerDepthFromCurrentAncestor;
385-
// And filter away elements that explicitly disable hover transfer
386-
return notUnequalDepths && !(item as HTMLElement).getAttribute?.("data-floating-menu-spawner")?.includes("no-hover-transfer");
388+
// And filter away descendants that explicitly disable hover transfer
389+
return notUnequalDepths && !(item instanceof HTMLElement && item.hasAttribute("data-block-hover-transfer"));
387390
});
388391
389392
// If none were found, widen the search by a level and keep trying (or stop looping if the root was reached)

frontend/src/components/views/Graph.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -501,7 +501,7 @@
501501
style:--data-color-dim={`var(--color-data-${(node.primaryOutput?.dataType || "General").toLowerCase()}-dim)`}
502502
style:--layer-area-width={layerAreaWidth}
503503
style:--node-chain-area-left-extension={layerChainWidth !== 0 ? layerChainWidth + 0.5 : 0}
504-
data-tooltip-label={node.displayName === node.reference ? node.displayName : `${node.displayName} (${node.reference})`}
504+
data-tooltip-label={node.displayName === node.reference || !node.reference ? node.displayName : `${node.displayName} (${node.reference})`}
505505
data-tooltip-description={`
506506
${(description || "").trim()}${editor.handle.inDevelopmentMode() ? `\n\nID: ${node.id}. Position: (${node.position.x}, ${node.position.y}).` : ""}
507507
`.trim()}
@@ -651,7 +651,7 @@
651651
style:--clip-path-id={`url(#${clipPathId})`}
652652
style:--data-color={`var(--color-data-${(node.primaryOutput?.dataType || "General").toLowerCase()})`}
653653
style:--data-color-dim={`var(--color-data-${(node.primaryOutput?.dataType || "General").toLowerCase()}-dim)`}
654-
data-tooltip-label={node.displayName === node.reference ? node.displayName : `${node.displayName} (${node.reference})`}
654+
data-tooltip-label={node.displayName === node.reference || !node.reference ? node.displayName : `${node.displayName} (${node.reference})`}
655655
data-tooltip-description={`
656656
${(description || "").trim()}${editor.handle.inDevelopmentMode() ? `\n\nID: ${node.id}. Position: (${node.position.x}, ${node.position.y}).` : ""}
657657
`.trim()}

frontend/src/components/widgets/WidgetSection.svelte

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@
5959
/>
6060
</button>
6161
{#if expanded}
62-
<LayoutCol class="body">
62+
<LayoutCol class="body" data-block-hover-transfer>
6363
{#each widgetData.layout as layoutGroup}
6464
{#if isWidgetSpanRow(layoutGroup)}
6565
<WidgetSpan widgetData={layoutGroup} {layoutTarget} />

frontend/src/components/widgets/buttons/TextButton.svelte

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,8 @@
7272
data-disabled={disabled || undefined}
7373
data-text-button
7474
tabindex={disabled ? -1 : 0}
75-
data-floating-menu-spawner={menuListChildrenExists ? "" : "no-hover-transfer"}
75+
data-floating-menu-spawner
76+
data-block-hover-transfer={menuListChildrenExists ? undefined : ""}
7677
on:click={onClick}
7778
>
7879
{#if icon}

frontend/src/components/widgets/inputs/WorkingColorsInput.svelte

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@
3737

3838
<LayoutCol class="working-colors-button">
3939
<LayoutRow class="primary swatch">
40-
<button on:click={clickPrimarySwatch} class:open={primaryOpen} style:--swatch-color={primary.toRgbaCSS()} data-floating-menu-spawner="no-hover-transfer" tabindex="0"></button>
40+
<button on:click={clickPrimarySwatch} class:open={primaryOpen} style:--swatch-color={primary.toRgbaCSS()} data-floating-menu-spawner data-block-hover-transfer tabindex="0"></button>
4141
<ColorPicker
4242
open={primaryOpen}
4343
on:open={({ detail }) => (primaryOpen = detail)}
@@ -47,7 +47,7 @@
4747
/>
4848
</LayoutRow>
4949
<LayoutRow class="secondary swatch">
50-
<button on:click={clickSecondarySwatch} class:open={secondaryOpen} style:--swatch-color={secondary.toRgbaCSS()} data-floating-menu-spawner="no-hover-transfer" tabindex="0"></button>
50+
<button on:click={clickSecondarySwatch} class:open={secondaryOpen} style:--swatch-color={secondary.toRgbaCSS()} data-floating-menu-spawner data-block-hover-transfer tabindex="0"></button>
5151
<ColorPicker
5252
open={secondaryOpen}
5353
on:open={({ detail }) => (secondaryOpen = detail)}

0 commit comments

Comments
 (0)