Skip to content

Add 'Align to Artboard' feature (issue #1720) #3981

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.

Sign up for GitHub

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

Open
afrdbaig7 wants to merge 1 commit into
base: master
Choose a base branch
from
+86 −7
Open

Add 'Align to Artboard' feature (issue #1720) #3981

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
6 changes: 6 additions & 0 deletions editor/src/messages/menu_bar/menu_bar_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::X,
aggregate: AlignAggregate::Min,
align_to_artboard: false,
}
.into()
})
Expand All @@ -365,6 +366,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::X,
aggregate: AlignAggregate::Center,
align_to_artboard: false,
}
.into()
})
Expand All @@ -376,6 +378,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::X,
aggregate: AlignAggregate::Max,
align_to_artboard: false,
}
.into()
})
Expand All @@ -389,6 +392,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::Y,
aggregate: AlignAggregate::Min,
align_to_artboard: false,
}
.into()
})
Expand All @@ -400,6 +404,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::Y,
aggregate: AlignAggregate::Center,
align_to_artboard: false,
}
.into()
})
Expand All @@ -411,6 +416,7 @@ impl LayoutHolder for MenuBarMessageHandler {
DocumentMessage::AlignSelectedLayers {
axis: AlignAxis::Y,
aggregate: AlignAggregate::Max,
align_to_artboard: false,
}
.into()
})
Expand Down
1 change: 1 addition & 0 deletions editor/src/messages/portfolio/document/document_message.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ pub enum DocumentMessage {
AlignSelectedLayers {
axis: AlignAxis,
aggregate: AlignAggregate,
align_to_artboard: bool,
},
RemoveArtboards,
ClearLayersPanel,
Expand Down
36 changes: 34 additions & 2 deletions editor/src/messages/portfolio/document/document_message_handler.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,12 +273,17 @@
let mut graph_operation_message_handler = GraphOperationMessageHandler {};
graph_operation_message_handler.process_message(message, responses, context);
}
DocumentMessage::AlignSelectedLayers { axis, aggregate } => {
DocumentMessage::AlignSelectedLayers { axis, aggregate, align_to_artboard } => {
let axis = match axis {
AlignAxis::X => DVec2::X,
AlignAxis::Y => DVec2::Y,
};
let Some(combined_box) = self.network_interface.selected_layers_artwork_bounding_box_viewport() else {
let combined_box = if align_to_artboard {
self.selected_layers_shared_artboard_bounding_box_viewport()
} else {
self.network_interface.selected_layers_artwork_bounding_box_viewport()
};
Comment on lines +281 to +285
Copy link
Copy Markdown
Contributor

@cubic-dev-ai cubic-dev-ai bot Mar 31, 2026

Choose a reason for hiding this comment

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

P2: Alignment to artboard now returns early when there’s no shared artboard, so the align command becomes a no-op if the selection spans multiple artboards. Consider falling back to selection bounds in that case so alignment still works even when the artboard-relative mode can’t resolve a shared artboard.

Prompt for AI agents 
Check if this issue is valid — if so, understand the root cause and fix it. At editor/src/messages/portfolio/document/document_message_handler.rs, line 281:

<comment>Alignment to artboard now returns early when there’s no shared artboard, so the align command becomes a no-op if the selection spans multiple artboards. Consider falling back to selection bounds in that case so alignment still works even when the artboard-relative mode can’t resolve a shared artboard.</comment>

<file context>
@@ -273,12 +273,17 @@ impl MessageHandler<DocumentMessage, DocumentMessageContext<'_>> for DocumentMes
 					AlignAxis::Y => DVec2::Y,
 				};
-				let Some(combined_box) = self.network_interface.selected_layers_artwork_bounding_box_viewport() else {
+				let combined_box = if align_to_artboard {
+					self.selected_layers_shared_artboard_bounding_box_viewport()
+				} else {
</file context>
Suggested change
let combined_box = if align_to_artboard {
self.selected_layers_shared_artboard_bounding_box_viewport()
} else {
self.network_interface.selected_layers_artwork_bounding_box_viewport()
};
let combined_box = if align_to_artboard {
self
.selected_layers_shared_artboard_bounding_box_viewport()
.or_else(|| self.network_interface.selected_layers_artwork_bounding_box_viewport())
} else {
self.network_interface.selected_layers_artwork_bounding_box_viewport()
};

Comment on lines +281 to +285
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

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

medium

The PR description states that when aligning to an artboard, it should fall back to selection bounds if no shared artboard exists. However, the current implementation will simply skip the alignment because selected_layers_shared_artboard_bounding_box_viewport() returns None, which causes the AlignSelectedLayers handler to return early.

To match the described behavior, you could use .or_else() to provide the fallback. For example:

let combined_box = if align_to_artboard {
    self.selected_layers_shared_artboard_bounding_box_viewport()
        .or_else(|| self.network_interface.selected_layers_artwork_bounding_box_viewport())
} else {
    self.network_interface.selected_layers_artwork_bounding_box_viewport()
};

If the current behavior of skipping alignment is intended, please consider updating the pull request description to reflect this.

let Some(combined_box) = combined_box else {
return;
};

Expand Down Expand Up @@ -1513,11 +1518,38 @@
}

impl DocumentMessageHandler {
<<<<<<< HEAD
/// Translates a viewport mouse position to a document-space transform, or uses the viewport center if no mouse position is given.
fn document_transform_from_mouse(&self, mouse: Option<(f64, f64)>, viewport: &ViewportMessageHandler) -> DAffine2 {
let viewport_pos: DVec2 = mouse.map_or_else(|| viewport.center_in_viewport_space().into_dvec2() + viewport.offset().into_dvec2(), |pos| pos.into());
let document_to_viewport = self.navigation_handler.calculate_offset_transform(viewport.center_in_viewport_space().into(), &self.document_ptz);
DAffine2::from_translation(document_to_viewport.inverse().transform_point2(viewport_pos - viewport.offset().into_dvec2()))
=======
fn selected_layers_shared_artboard_bounding_box_viewport(&self) -> Option<[DVec2; 2]> {
let selected_nodes = self.network_interface.selected_nodes();
let mut selected_layers = selected_nodes.selected_unlocked_layers(&self.network_interface).peekable();
selected_layers.peek()?;

let mut shared_artboard = None;

for layer in selected_layers {
let layer_artboard = if self.network_interface.is_artboard(&layer.to_node(), &[]) {
Some(layer)
} else {
layer
.ancestors(self.metadata())
.find(|ancestor| *ancestor != LayerNodeIdentifier::ROOT_PARENT && self.network_interface.is_artboard(&ancestor.to_node(), &[]))
};

match (shared_artboard, layer_artboard) {
(None, Some(artboard)) => shared_artboard = Some(artboard),
(Some(shared_artboard), Some(artboard)) if shared_artboard == artboard => {}
_ => return None,
}
}

shared_artboard.and_then(|artboard| self.metadata().bounding_box_viewport(artboard))
>>>>>>> 6525a73f6 (Add 'Align to Artboard' feature (issue #1720))

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / test 

unterminated character literal

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / test 

prefix `Artboard` is unknown

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / rust-fmt 

unterminated character literal

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / rust-fmt 

prefix `Artboard` is unknown

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web 

unterminated character literal

Check failure on line 1552 in editor/src/messages/portfolio/document/document_message_handler.rs

View workflow job for this annotation

GitHub Actions / build / web 

prefix `Artboard` is unknown
}
Comment on lines +1521 to 1553
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

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

critical

This file contains unresolved merge conflict markers (<<<<<<<, =======, >>>>>>>). This will prevent the code from compiling and must be resolved.

It looks like you intended to add the selected_layers_shared_artboard_bounding_box_viewport function while keeping the existing document_transform_from_mouse function.

	/// Translates a viewport mouse position to a document-space transform, or uses the viewport center if no mouse position is given.
	fn document_transform_from_mouse(&self, mouse: Option<(f64, f64)>, viewport: &ViewportMessageHandler) -> DAffine2 {
		let viewport_pos: DVec2 = mouse.map_or_else(|| viewport.center_in_viewport_space().into_dvec2() + viewport.offset().into_dvec2(), |pos| pos.into());
		let document_to_viewport = self.navigation_handler.calculate_offset_transform(viewport.center_in_viewport_space().into(), &self.document_ptz);
		DAffine2::from_translation(document_to_viewport.inverse().transform_point2(viewport_pos - viewport.offset().into_dvec2()))
	}

	fn selected_layers_shared_artboard_bounding_box_viewport(&self) -> Option<[DVec2; 2]> {
		let selected_nodes = self.network_interface.selected_nodes();
		let mut selected_layers = selected_nodes.selected_unlocked_layers(&self.network_interface).peekable();
		selected_layers.peek()?;

		let mut shared_artboard = None;

		for layer in selected_layers {
			let layer_artboard = if self.network_interface.is_artboard(&layer.to_node(), &[]) {
				Some(layer)
			} else {
				layer
					.ancestors(self.metadata())
					.find(|ancestor| *ancestor != LayerNodeIdentifier::ROOT_PARENT && self.network_interface.is_artboard(&ancestor.to_node(), &[]))
			};

			match (shared_artboard, layer_artboard) {
				(None, Some(artboard)) => shared_artboard = Some(artboard),
				(Some(shared_artboard), Some(artboard)) if shared_artboard == artboard => {}
				_ => return None,
			}
		}

		shared_artboard.and_then(|artboard| self.metadata().bounding_box_viewport(artboard))
	}


/// Runs an intersection test with all layers and a viewport space quad
Expand Down
50 changes: 45 additions & 5 deletions editor/src/messages/tool/tool_messages/select_tool.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,9 @@ pub enum SelectToolMessage {
SelectOptions {
options: SelectOptionsUpdate,
},
SetAlignToArtboard {
align_to_artboard: bool,
},
SetPivot {
position: ReferencePoint,
},
Expand Down Expand Up @@ -159,7 +162,7 @@ impl SelectTool {
.widget_instance()
}

fn alignment_widgets(&self, disabled: bool) -> impl Iterator<Item = WidgetInstance> + use<> {
fn alignment_widgets(&self, disabled: bool, align_to_artboard: bool) -> impl Iterator<Item = WidgetInstance> + use<> {
[AlignAxis::X, AlignAxis::Y]
.into_iter()
.flat_map(|axis| [(axis, AlignAggregate::Min), (axis, AlignAggregate::Center), (axis, AlignAggregate::Max)])
Expand All @@ -174,12 +177,18 @@ impl SelectTool {
};
IconButton::new(icon, 24)
.tooltip_label(label)
.on_update(move |_| DocumentMessage::AlignSelectedLayers { axis, aggregate }.into())
.on_update(move |_| DocumentMessage::AlignSelectedLayers { axis, aggregate, align_to_artboard }.into())
.disabled(disabled)
.widget_instance()
})
}

fn should_refresh_align_to_artboard(&mut self) -> bool {
let align_to_artboard_changed = self.tool_data.align_to_artboard_changed;
self.tool_data.align_to_artboard_changed = false;
align_to_artboard_changed
}
Comment on lines +186 to +190
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

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

medium

This function can be simplified by using std::mem::take. This is more idiomatic and concise for swapping a value with its default.

	fn should_refresh_align_to_artboard(&mut self) -> bool {
		std::mem::take(&mut self.tool_data.align_to_artboard_changed)
	}


fn flip_widgets(&self, disabled: bool) -> impl Iterator<Item = WidgetInstance> + use<> {
[(FlipAxis::X, "FlipHorizontal", "Flip Horizontal"), (FlipAxis::Y, "FlipVertical", "Flip Vertical")]
.into_iter()
Expand Down Expand Up @@ -252,9 +261,33 @@ impl LayoutHolder for SelectTool {
}

// Align
let disabled = self.tool_data.selected_layers_count < 2;
let align_to_artboard = self.tool_data.align_to_artboard;
let disabled = self.tool_data.selected_layers_count == 0 || (!align_to_artboard && self.tool_data.selected_layers_count < 2);
let align_to_artboard_checkbox_id = CheckboxId::new();
widgets.push(Separator::new(SeparatorStyle::Unrelated).widget_instance());
widgets.extend(self.alignment_widgets(disabled));
widgets.extend(self.alignment_widgets(disabled, align_to_artboard));
widgets.push(
PopoverButton::new()
.icon("AlignVerticalCenter")
.tooltip_label("Alignment Options")
.tooltip_description("Change how alignment uses the selection bounds or artboard bounds.")
.popover_min_width(Some(190))
.popover_layout(Layout(vec![LayoutGroup::row(vec![
CheckboxInput::new(align_to_artboard)
.for_label(align_to_artboard_checkbox_id)
.tooltip_label("To Artboard")
.tooltip_description("Align selected layers to their shared artboard instead of the selection bounds.")
.on_update(|input: &CheckboxInput| SelectToolMessage::SetAlignToArtboard { align_to_artboard: input.checked }.into())
.widget_instance(),
TextLabel::new("To Artboard")
.tooltip_label("To Artboard")
.tooltip_description("Align selected layers to their shared artboard instead of the selection bounds.")
.for_checkbox(align_to_artboard_checkbox_id)
.widget_instance(),
Comment on lines +282 to +286
Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist bot Mar 31, 2026

Choose a reason for hiding this comment

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

medium

The tooltip_label and tooltip_description are duplicated for both the CheckboxInput and its associated TextLabel. Since the label is linked to the checkbox with for_checkbox, the tooltip will be shown when hovering over either widget. You can remove the redundant tooltips from the TextLabel to simplify the code.

					TextLabel::new("To Artboard")
						.for_checkbox(align_to_artboard_checkbox_id)
						.widget_instance(),

])]))
.disabled(self.tool_data.selected_layers_count == 0)
.widget_instance(),
);

// Flip
let disabled = self.tool_data.selected_layers_count == 0;
Expand Down Expand Up @@ -312,7 +345,7 @@ impl<'a> MessageHandler<ToolMessage, &mut ToolActionMessageContext<'a>> for Sele

self.fsm_state.process_event(message, &mut self.tool_data, context, &(), responses, false);

if self.tool_data.pivot_gizmo.pivot.should_refresh_pivot_position() || self.tool_data.selected_layers_changed || redraw_reference_pivot {
if self.tool_data.pivot_gizmo.pivot.should_refresh_pivot_position() || self.should_refresh_align_to_artboard() || self.tool_data.selected_layers_changed || redraw_reference_pivot {
// Send the layout containing the updated pivot position (a bit ugly to do it here not in the fsm but that doesn't have SelectTool)
self.send_layout(responses, LayoutTarget::ToolOptions);
self.tool_data.selected_layers_changed = false;
Expand Down Expand Up @@ -406,6 +439,8 @@ struct SelectToolData {
snap_candidates: Vec<SnapCandidatePoint>,
auto_panning: AutoPanning,
drag_start_center: ViewportPosition,
align_to_artboard: bool,
align_to_artboard_changed: bool,
}

impl SelectToolData {
Expand Down Expand Up @@ -611,6 +646,11 @@ impl Fsm for SelectToolFsmState {

let ToolMessage::Select(event) = event else { return self };
match (self, event) {
(_, SelectToolMessage::SetAlignToArtboard { align_to_artboard }) => {
tool_data.align_to_artboard = align_to_artboard;
tool_data.align_to_artboard_changed = true;
self
}
(_, SelectToolMessage::Overlays { context: mut overlay_context }) => {
tool_data.snap_manager.draw_overlays(SnapData::new(document, input, viewport), &mut overlay_context);

Expand Down
Loading