mirror of
https://github.com/slint-ui/slint.git
synced 2025-11-03 13:23:00 +00:00
Add a compiler warning when using padding outside of layout
The padding property is accepted but doesn't have an effect. It was meant for future use but didn't get any use. And people get confuse when they set it. (See #8829) Closes: #8829 Closes: #6288
This commit is contained in:
parent
bf8729d7f3
commit
a007ca8c60
2 changed files with 81 additions and 20 deletions
|
|
@ -46,14 +46,14 @@ pub fn lower_layouts(
|
||||||
|
|
||||||
recurse_elem_including_sub_components(component, &(), &mut |elem, _| {
|
recurse_elem_including_sub_components(component, &(), &mut |elem, _| {
|
||||||
let component = elem.borrow().enclosing_component.upgrade().unwrap();
|
let component = elem.borrow().enclosing_component.upgrade().unwrap();
|
||||||
lower_element_layout(
|
let is_layout = lower_element_layout(
|
||||||
&component,
|
&component,
|
||||||
elem,
|
elem,
|
||||||
&type_loader.global_type_registry.borrow(),
|
&type_loader.global_type_registry.borrow(),
|
||||||
style_metrics,
|
style_metrics,
|
||||||
diag,
|
diag,
|
||||||
);
|
);
|
||||||
check_no_layout_properties(elem, diag);
|
check_no_layout_properties(elem, is_layout, diag);
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
@ -81,17 +81,19 @@ fn check_preferred_size_100(elem: &ElementRc, prop: &str, diag: &mut BuildDiagno
|
||||||
false
|
false
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// If the element is a layout, lower it to a Rectangle, and set the geometry property of the element inside it.
|
||||||
|
/// Returns true if the element was a layout and has been lowered
|
||||||
fn lower_element_layout(
|
fn lower_element_layout(
|
||||||
component: &Rc<Component>,
|
component: &Rc<Component>,
|
||||||
elem: &ElementRc,
|
elem: &ElementRc,
|
||||||
type_register: &TypeRegister,
|
type_register: &TypeRegister,
|
||||||
style_metrics: &Rc<Component>,
|
style_metrics: &Rc<Component>,
|
||||||
diag: &mut BuildDiagnostics,
|
diag: &mut BuildDiagnostics,
|
||||||
) {
|
) -> bool {
|
||||||
let base_type = if let ElementType::Builtin(base_type) = &elem.borrow().base_type {
|
let base_type = if let ElementType::Builtin(base_type) = &elem.borrow().base_type {
|
||||||
base_type.clone()
|
base_type.clone()
|
||||||
} else {
|
} else {
|
||||||
return;
|
return false;
|
||||||
};
|
};
|
||||||
match base_type.name.as_str() {
|
match base_type.name.as_str() {
|
||||||
"Row" => {
|
"Row" => {
|
||||||
|
|
@ -106,19 +108,18 @@ fn lower_element_layout(
|
||||||
.is_some_and(|e| e.borrow().repeated.is_some()),
|
.is_some_and(|e| e.borrow().repeated.is_some()),
|
||||||
"Error should have been caught at element lookup time"
|
"Error should have been caught at element lookup time"
|
||||||
);
|
);
|
||||||
return;
|
return false;
|
||||||
}
|
}
|
||||||
"GridLayout" => lower_grid_layout(component, elem, diag, type_register),
|
"GridLayout" => lower_grid_layout(component, elem, diag, type_register),
|
||||||
"HorizontalLayout" => lower_box_layout(elem, diag, Orientation::Horizontal),
|
"HorizontalLayout" => lower_box_layout(elem, diag, Orientation::Horizontal),
|
||||||
"VerticalLayout" => lower_box_layout(elem, diag, Orientation::Vertical),
|
"VerticalLayout" => lower_box_layout(elem, diag, Orientation::Vertical),
|
||||||
"Dialog" => {
|
"Dialog" => {
|
||||||
lower_dialog_layout(elem, style_metrics, diag);
|
lower_dialog_layout(elem, style_metrics, diag);
|
||||||
return; // the Dialog stays in the tree as a Dialog
|
return false; // the Dialog stays in the tree as a Dialog
|
||||||
}
|
}
|
||||||
_ => return,
|
_ => return false,
|
||||||
};
|
};
|
||||||
|
|
||||||
{
|
|
||||||
let mut elem = elem.borrow_mut();
|
let mut elem = elem.borrow_mut();
|
||||||
let elem = &mut *elem;
|
let elem = &mut *elem;
|
||||||
let prev_base = std::mem::replace(&mut elem.base_type, type_register.empty_type());
|
let prev_base = std::mem::replace(&mut elem.base_type, type_register.empty_type());
|
||||||
|
|
@ -131,7 +132,8 @@ fn lower_element_layout(
|
||||||
elem.property_declarations.insert(p, ty.into());
|
elem.property_declarations.insert(p, ty.into());
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
|
||||||
|
true
|
||||||
}
|
}
|
||||||
|
|
||||||
fn lower_grid_layout(
|
fn lower_grid_layout(
|
||||||
|
|
@ -821,7 +823,7 @@ fn eval_const_expr(
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Checks that there is grid-layout specific properties left
|
/// Checks that there is grid-layout specific properties left
|
||||||
fn check_no_layout_properties(item: &ElementRc, diag: &mut BuildDiagnostics) {
|
fn check_no_layout_properties(item: &ElementRc, is_layout: bool, diag: &mut BuildDiagnostics) {
|
||||||
for (prop, expr) in item.borrow().bindings.iter() {
|
for (prop, expr) in item.borrow().bindings.iter() {
|
||||||
if matches!(prop.as_ref(), "col" | "row" | "colspan" | "rowspan") {
|
if matches!(prop.as_ref(), "col" | "row" | "colspan" | "rowspan") {
|
||||||
diag.push_error(format!("{prop} used outside of a GridLayout"), &*expr.borrow());
|
diag.push_error(format!("{prop} used outside of a GridLayout"), &*expr.borrow());
|
||||||
|
|
@ -829,6 +831,17 @@ fn check_no_layout_properties(item: &ElementRc, diag: &mut BuildDiagnostics) {
|
||||||
if matches!(prop.as_ref(), "dialog-button-role") {
|
if matches!(prop.as_ref(), "dialog-button-role") {
|
||||||
diag.push_error(format!("{prop} used outside of a Dialog"), &*expr.borrow());
|
diag.push_error(format!("{prop} used outside of a Dialog"), &*expr.borrow());
|
||||||
}
|
}
|
||||||
|
if !is_layout
|
||||||
|
&& matches!(
|
||||||
|
prop.as_ref(),
|
||||||
|
"padding" | "padding-left" | "padding-right" | "padding-top" | "padding-bottom"
|
||||||
|
)
|
||||||
|
{
|
||||||
|
diag.push_warning(
|
||||||
|
format!("{prop} only has effect on layout elements"),
|
||||||
|
&*expr.borrow(),
|
||||||
|
);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
48
internal/compiler/tests/syntax/layout/padding.slint
Normal file
48
internal/compiler/tests/syntax/layout/padding.slint
Normal file
|
|
@ -0,0 +1,48 @@
|
||||||
|
// Copyright © SixtyFPS GmbH <info@slint.dev>
|
||||||
|
// SPDX-License-Identifier: GPL-3.0-only OR LicenseRef-Slint-Royalty-free-2.0 OR LicenseRef-Slint-Software-3.0
|
||||||
|
|
||||||
|
component MyLayout inherits VerticalLayout {
|
||||||
|
padding: 5px;
|
||||||
|
Rectangle { padding: 5px; }
|
||||||
|
// ^warning{padding only has effect on layout elements}
|
||||||
|
}
|
||||||
|
|
||||||
|
export component Test {
|
||||||
|
|
||||||
|
padding: 8px;
|
||||||
|
// ^warning{padding only has effect on layout elements}
|
||||||
|
|
||||||
|
padding-bottom: 2px;
|
||||||
|
// ^warning{padding-bottom only has effect on layout elements}
|
||||||
|
|
||||||
|
Rectangle {
|
||||||
|
padding-top: 2px;
|
||||||
|
// ^warning{padding-top only has effect on layout elements}
|
||||||
|
GridLayout {
|
||||||
|
padding: 5px;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
HorizontalLayout {
|
||||||
|
padding-left: 5px;
|
||||||
|
Rectangle {
|
||||||
|
padding: 5px;
|
||||||
|
// ^warning{padding only has effect on layout elements}
|
||||||
|
Rectangle {
|
||||||
|
padding: 5px;
|
||||||
|
// ^warning{padding only has effect on layout elements}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
MyLayout {
|
||||||
|
padding-left: 10px;
|
||||||
|
Rectangle {
|
||||||
|
padding: -5px;
|
||||||
|
// ^warning{padding only has effect on layout elements}
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
|
||||||
|
}
|
||||||
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue