Name a lone control after its operation, so three cannot all read "Amount"
Benchmarks / CPU and I/O (per commit) (push) Successful in 10m31s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 1h15m4s
Build and test / Layer separation (push) Successful in 37s
Traceability / Requirement traces (push) Failing after 1m28s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Build and test / Android (aarch64) (push) Successful in 1h3m13s
Benchmarks / CPU and I/O (per commit) (push) Successful in 10m31s
Benchmarks / Frame budget (on demand) (push) Skipped
Build and test / Desktop (Linux) (push) Failing after 1h15m4s
Build and test / Layer separation (push) Successful in 37s
Traceability / Requirement traces (push) Failing after 1m28s
🐳 Android image / Build and push (push) Successful in 2s
Build and test / android-image (push) Successful in 2s
Build and test / Android (aarch64) (push) Successful in 1h3m13s
The Detail group ended with three consecutive sliders labelled "Amount" and nothing to tell them apart. They are dehaze, clarity and texture: each declares exactly one parameter, and `ops/README.md` tells an author to reach for the `amount` kind first, so all three named it the same thing. The panel withholds a heading from a group of one, and the reasoning it gives is sound — a lone control names itself, and the group reset it loses costs nothing because the slider already resets on double-click. But that argument rests on the parameter being named after what it does. It holds for exposure, contrast, vibrance, saturation and brilliance, whose single parameter shares the operation's name, and it fails for the three whose parameter is called after its kind rather than its subject. So a lone parameter now takes its operation's label — the name the withheld heading would have carried. For the five that already agreed, nothing changes. Found on the tablet, and only there: every row was correct, every label resolved, and the panel was still unusable. The test that guards it asserts over the real chain rather than a fixture, because the fault was a property of what is actually declared — a fixture would have had to be written to reproduce it, and would then only have proved itself.
This commit is contained in:
+34
-33
File diff suppressed because one or more lines are too long
@@ -1398,8 +1398,23 @@ pub(crate) fn rows_filtered(
|
|||||||
// because its aspect is already written above the run it sits
|
// because its aspect is already written above the run it sits
|
||||||
// in. Unfaceted parameters keep their own label, which is
|
// in. Unfaceted parameters keep their own label, which is
|
||||||
// every operation but the mixer.
|
// every operation but the mixer.
|
||||||
|
//
|
||||||
|
// Except when the parameter is the operation's only one. The panel
|
||||||
|
// draws no heading over a group of one, on the argument that a
|
||||||
|
// lone control names itself — and that holds only while the
|
||||||
|
// parameter is named after what it does. Three operations declare
|
||||||
|
// a single parameter called `amount`, which is the name
|
||||||
|
// `ops/README.md` tells an author to reach for first, and they
|
||||||
|
// arrived in the panel as three consecutive sliders all labelled
|
||||||
|
// "Amount" with nothing to tell them apart.
|
||||||
|
//
|
||||||
|
// So a lone parameter is titled by its operation. That is the name
|
||||||
|
// the missing heading would have carried, and for the operations
|
||||||
|
// whose one parameter already shares the operation's name it reads
|
||||||
|
// exactly as it did before.
|
||||||
let param_label = match &p.facet {
|
let param_label = match &p.facet {
|
||||||
Some(f) => labels::resolve(f.subject.0),
|
Some(f) => labels::resolve(f.subject.0),
|
||||||
|
None if op.params.len() == 1 => labels::resolve(op.label.0),
|
||||||
None => labels::resolve(p.label.0),
|
None => labels::resolve(p.label.0),
|
||||||
};
|
};
|
||||||
let aspect = p.facet.as_ref().map(|f| f.aspect.0);
|
let aspect = p.facet.as_ref().map(|f| f.aspect.0);
|
||||||
@@ -7337,6 +7352,56 @@ mod tests {
|
|||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// TRACES: FR-DEV-3
|
||||||
|
/// A lone parameter is titled by its operation, so several cannot collide.
|
||||||
|
///
|
||||||
|
/// The panel draws no heading over a group of one, on the argument that a
|
||||||
|
/// lone control names itself. Three operations declare a single parameter
|
||||||
|
/// called `amount` — the name `ops/README.md` tells an author to reach for
|
||||||
|
/// first — and they reached the Detail group as three consecutive sliders
|
||||||
|
/// all reading "Amount", which is a panel a photographer cannot use.
|
||||||
|
///
|
||||||
|
/// Asserted over the real chain rather than a fixture, because the failure
|
||||||
|
/// was a property of what is actually declared: a fixture would have to be
|
||||||
|
/// written to reproduce it and would then only prove itself.
|
||||||
|
#[test]
|
||||||
|
fn a_lone_parameter_is_named_after_its_operation() {
|
||||||
|
let graph = EditGraph::default_chain();
|
||||||
|
let caps = graph.capabilities();
|
||||||
|
let rows = rows_filtered(&caps, |_| true, 0);
|
||||||
|
|
||||||
|
for (i, cap) in caps.iter().enumerate() {
|
||||||
|
if cap.params.len() != 1 || cap.presentation.is_some() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
let Some(row) = rows.iter().find(|r| r.op_index as usize == i) else {
|
||||||
|
continue;
|
||||||
|
};
|
||||||
|
assert_eq!(
|
||||||
|
row.param_label,
|
||||||
|
labels::resolve(cap.label.0).as_str(),
|
||||||
|
"a lone parameter must carry its operation's name, or the \
|
||||||
|
heading the panel withheld takes the name with it"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
|
// And the specific collision that started this: no two rows drawn
|
||||||
|
// without a heading may read the same.
|
||||||
|
let bare: Vec<_> = rows
|
||||||
|
.iter()
|
||||||
|
.filter(|r| r.group_len == 1)
|
||||||
|
.map(|r| r.param_label.to_string())
|
||||||
|
.collect();
|
||||||
|
let mut unique = bare.clone();
|
||||||
|
unique.sort();
|
||||||
|
unique.dedup();
|
||||||
|
assert_eq!(
|
||||||
|
bare.len(),
|
||||||
|
unique.len(),
|
||||||
|
"two headingless rows share a label: {bare:?}"
|
||||||
|
);
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn switching_curve_repoints_the_row() {
|
fn switching_curve_repoints_the_row() {
|
||||||
use slint::Model as _;
|
use slint::Model as _;
|
||||||
|
|||||||
Reference in New Issue
Block a user