Create Task
Maniphest T1236

Idea: Add support for units with Int values
Closed, WontfixPublic
Actions

Description

Double value has the "unit" option using the internal KPropertyUnit class.
Ints could have the same logic: values could be of Double type, conversions to points could be required.

Example code:

const int cm = 1; // 28.3465058 points
const qreal points = KPropertyUnit(KPropertyUnit::Centimeter).fromUserValue(cm);
// Note how we have to specify type KProperty::Int explictly otherwise KProperty::Auto will lead to detection of Double type
m_set.addProperty(p = new KProperty("cm", points, "Double cm", QString(), KProperty::Int), group);
p->setOption("unit", "cm");

See also T1235.

Related Objects

staniek created this task.Dec 28 2015, 3:10 PM
staniek updated the task description. (Show Details)
staniek raised the priority of this task from to Wishlist.
staniek added a project: KProperty.
staniek moved this task to Backlog on the KProperty board.
staniek updated the task description. (Show Details)
staniek added a subscriber: staniek.
staniek added a subscriber: piggz.Dec 28 2015, 7:37 PM

@piggz Any comments?

piggz added a comment.Dec 30 2015, 4:41 PM

what would the use case be? the precision would be terrible i think ;)

staniek added a comment.Dec 30 2015, 4:44 PM

Someone sets unit == "mm" and wants precision == 0. Then the Integer editor makes sense.

piggz added a comment.Dec 30 2015, 4:46 PM

What is the vale returned by the property, an integer number of points?

staniek added a comment.Dec 30 2015, 5:28 PM

Good question. Depends on the needs.

Perhaps it's even more subtle.

Regardless what we need (double or integer "visible values") it's not clear if users always want the value() method to return values in points as we currently do for the double type. What's more intuitive?

Especially for integer type it's intuitive to have values in "mm" returned by value() when unit == "mm". For consistency, in case of double types, we could switch to that behaviour and maybe have both "unit" and "visibleUnit" options, where "visibleUnit" is the same what we currently know as "unit".

Opinions?

staniek added a comment.Dec 30 2015, 5:29 PM

Actually such a deeper incompatible change is still possible before the release.

staniek added a comment.Dec 30 2015, 5:29 PM

Actually such a deeper incompatible change is still possible before the release.

staniek closed this task as Wontfix.Apr 12 2017, 6:54 PM

Dropped, looks that precision == 0 is enough, having special case for integers complicates code and docs.